From 7280905ee3f7ac4ef4a8bad2be94ceeea5a8dc83 Mon Sep 17 00:00:00 2001 From: demolaf Date: Mon, 20 Jul 2026 14:20:17 +0100 Subject: [PATCH 1/5] fix(firestore): catch InterruptedException in FirestorePagingSource --- .../paging/FirestorePagingSourceTest.java | 31 +++++++++++++++++++ .../paging/FirestorePagingSource.java | 12 ++++++- 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/firestore/src/androidTest/java/com/firebase/ui/firestore/paging/FirestorePagingSourceTest.java b/firestore/src/androidTest/java/com/firebase/ui/firestore/paging/FirestorePagingSourceTest.java index 3cec97075..3f28ae451 100644 --- a/firestore/src/androidTest/java/com/firebase/ui/firestore/paging/FirestorePagingSourceTest.java +++ b/firestore/src/androidTest/java/com/firebase/ui/firestore/paging/FirestorePagingSourceTest.java @@ -1,5 +1,6 @@ package com.firebase.ui.firestore.paging; +import com.google.android.gms.tasks.TaskCompletionSource; import com.google.android.gms.tasks.Tasks; import com.google.firebase.firestore.DocumentSnapshot; import com.google.firebase.firestore.Query; @@ -14,12 +15,15 @@ import java.util.ArrayList; import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; import androidx.paging.PagingSource; import androidx.paging.PagingSource.LoadParams.Append; import androidx.paging.PagingSource.LoadParams.Refresh; import androidx.paging.PagingSource.LoadResult.Page; import androidx.test.ext.junit.runners.AndroidJUnit4; +import io.reactivex.rxjava3.disposables.Disposable; +import io.reactivex.rxjava3.plugins.RxJavaPlugins; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -105,6 +109,33 @@ public void testLoadAfter_failure() { assertEquals(expected, actual); } + @Test + public void testLoadSingle_interruptedWhileDisposed_doesNotThrowUndeliverableException() + throws InterruptedException { + FirestorePagingSource pagingSource = new FirestorePagingSource(mMockQuery, Source.DEFAULT); + TaskCompletionSource taskCompletionSource = new TaskCompletionSource<>(); + when(mMockQuery.get(Source.DEFAULT)).thenReturn(taskCompletionSource.getTask()); + + List undeliverableErrors = new CopyOnWriteArrayList<>(); + RxJavaPlugins.setErrorHandler(undeliverableErrors::add); + try { + Refresh refreshRequest = new Refresh<>(null, 2, false); + Disposable disposable = pagingSource.loadSingle(refreshRequest) + .subscribe(result -> { }, error -> { }); + + Thread.sleep(300); + disposable.dispose(); + Thread.sleep(300); + + assertTrue( + "Interruption during an in-flight load must not surface as an " + + "undeliverable RxJava exception: " + undeliverableErrors, + undeliverableErrors.isEmpty()); + } finally { + RxJavaPlugins.setErrorHandler(null); + } + } + private void initMockQuery() { when(mMockQuery.startAfter(any(DocumentSnapshot.class))).thenReturn(mMockQuery); when(mMockQuery.endBefore(any(DocumentSnapshot.class))).thenReturn(mMockQuery); diff --git a/firestore/src/main/java/com/firebase/ui/firestore/paging/FirestorePagingSource.java b/firestore/src/main/java/com/firebase/ui/firestore/paging/FirestorePagingSource.java index d10dd2b1b..f23301669 100644 --- a/firestore/src/main/java/com/firebase/ui/firestore/paging/FirestorePagingSource.java +++ b/firestore/src/main/java/com/firebase/ui/firestore/paging/FirestorePagingSource.java @@ -1,5 +1,7 @@ package com.firebase.ui.firestore.paging; +import android.util.Log; + import com.google.android.gms.tasks.Task; import com.google.android.gms.tasks.Tasks; import com.google.firebase.firestore.DocumentSnapshot; @@ -19,6 +21,8 @@ public class FirestorePagingSource extends RxPagingSource { + private static final String TAG = "FirestorePagingSource"; + private final Query mQuery; private final Source mSource; @@ -54,8 +58,14 @@ public Single> loadSingle(@NonNull LoadPar // Only throw a new Exception when the original // Throwable cannot be cast to Exception throw new Exception(e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + return new LoadResult.Error(e); } - }).subscribeOn(Schedulers.io()).onErrorReturn(LoadResult.Error::new); + }).subscribeOn(Schedulers.io()).onErrorReturn(e -> { + Log.e(TAG, "FirestorePagingSource load failed", e); + return new LoadResult.Error<>(e); + }); } private LoadResult toLoadResult( From e92ecd73d954e8fd5e1f677223321b30cb3cf460 Mon Sep 17 00:00:00 2001 From: demolaf Date: Tue, 21 Jul 2026 12:06:05 +0100 Subject: [PATCH 2/5] fix(firestore): drop load error logging and restore rx error handler in test --- .../ui/firestore/paging/FirestorePagingSourceTest.java | 4 +++- .../ui/firestore/paging/FirestorePagingSource.java | 9 +-------- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/firestore/src/androidTest/java/com/firebase/ui/firestore/paging/FirestorePagingSourceTest.java b/firestore/src/androidTest/java/com/firebase/ui/firestore/paging/FirestorePagingSourceTest.java index 3f28ae451..9ba76d27b 100644 --- a/firestore/src/androidTest/java/com/firebase/ui/firestore/paging/FirestorePagingSourceTest.java +++ b/firestore/src/androidTest/java/com/firebase/ui/firestore/paging/FirestorePagingSourceTest.java @@ -23,6 +23,7 @@ import androidx.paging.PagingSource.LoadResult.Page; import androidx.test.ext.junit.runners.AndroidJUnit4; import io.reactivex.rxjava3.disposables.Disposable; +import io.reactivex.rxjava3.functions.Consumer; import io.reactivex.rxjava3.plugins.RxJavaPlugins; import static org.junit.Assert.assertEquals; @@ -117,6 +118,7 @@ public void testLoadSingle_interruptedWhileDisposed_doesNotThrowUndeliverableExc when(mMockQuery.get(Source.DEFAULT)).thenReturn(taskCompletionSource.getTask()); List undeliverableErrors = new CopyOnWriteArrayList<>(); + Consumer originalErrorHandler = RxJavaPlugins.getErrorHandler(); RxJavaPlugins.setErrorHandler(undeliverableErrors::add); try { Refresh refreshRequest = new Refresh<>(null, 2, false); @@ -132,7 +134,7 @@ public void testLoadSingle_interruptedWhileDisposed_doesNotThrowUndeliverableExc + "undeliverable RxJava exception: " + undeliverableErrors, undeliverableErrors.isEmpty()); } finally { - RxJavaPlugins.setErrorHandler(null); + RxJavaPlugins.setErrorHandler(originalErrorHandler); } } diff --git a/firestore/src/main/java/com/firebase/ui/firestore/paging/FirestorePagingSource.java b/firestore/src/main/java/com/firebase/ui/firestore/paging/FirestorePagingSource.java index f23301669..ca96fb90a 100644 --- a/firestore/src/main/java/com/firebase/ui/firestore/paging/FirestorePagingSource.java +++ b/firestore/src/main/java/com/firebase/ui/firestore/paging/FirestorePagingSource.java @@ -1,7 +1,5 @@ package com.firebase.ui.firestore.paging; -import android.util.Log; - import com.google.android.gms.tasks.Task; import com.google.android.gms.tasks.Tasks; import com.google.firebase.firestore.DocumentSnapshot; @@ -21,8 +19,6 @@ public class FirestorePagingSource extends RxPagingSource { - private static final String TAG = "FirestorePagingSource"; - private final Query mQuery; private final Source mSource; @@ -62,10 +58,7 @@ public Single> loadSingle(@NonNull LoadPar Thread.currentThread().interrupt(); return new LoadResult.Error(e); } - }).subscribeOn(Schedulers.io()).onErrorReturn(e -> { - Log.e(TAG, "FirestorePagingSource load failed", e); - return new LoadResult.Error<>(e); - }); + }).subscribeOn(Schedulers.io()).onErrorReturn(LoadResult.Error::new); } private LoadResult toLoadResult( From 0b71e8e379ebe17b485018b62067e6ae4920e35f Mon Sep 17 00:00:00 2001 From: demolaf Date: Tue, 21 Jul 2026 12:56:03 +0100 Subject: [PATCH 3/5] fix(firestore): return LoadResult.Error for every load failure instead of throwing --- .../ui/firestore/paging/FirestorePagingSource.java | 12 +++++------- .../firestore/paging/FirestorePagingSourceTest.java | 0 2 files changed, 5 insertions(+), 7 deletions(-) rename firestore/src/{androidTest => test}/java/com/firebase/ui/firestore/paging/FirestorePagingSourceTest.java (100%) diff --git a/firestore/src/main/java/com/firebase/ui/firestore/paging/FirestorePagingSource.java b/firestore/src/main/java/com/firebase/ui/firestore/paging/FirestorePagingSource.java index ca96fb90a..4a6942bf9 100644 --- a/firestore/src/main/java/com/firebase/ui/firestore/paging/FirestorePagingSource.java +++ b/firestore/src/main/java/com/firebase/ui/firestore/paging/FirestorePagingSource.java @@ -47,16 +47,14 @@ public Single> loadSingle(@NonNull LoadPar } return toLoadResult(snapshot.getDocuments(), nextPage); } catch (ExecutionException e) { - if (e.getCause() instanceof Exception) { - // throw the original Exception - throw (Exception) e.getCause(); - } - // Only throw a new Exception when the original - // Throwable cannot be cast to Exception - throw new Exception(e); + // Report the original cause rather than the ExecutionException wrapper. + Throwable cause = e.getCause(); + return new LoadResult.Error(cause == null ? e : cause); } catch (InterruptedException e) { Thread.currentThread().interrupt(); return new LoadResult.Error(e); + } catch (Exception e) { + return new LoadResult.Error(e); } }).subscribeOn(Schedulers.io()).onErrorReturn(LoadResult.Error::new); } diff --git a/firestore/src/androidTest/java/com/firebase/ui/firestore/paging/FirestorePagingSourceTest.java b/firestore/src/test/java/com/firebase/ui/firestore/paging/FirestorePagingSourceTest.java similarity index 100% rename from firestore/src/androidTest/java/com/firebase/ui/firestore/paging/FirestorePagingSourceTest.java rename to firestore/src/test/java/com/firebase/ui/firestore/paging/FirestorePagingSourceTest.java From 96b6c14b14379cabaa0394f194341525557d971f Mon Sep 17 00:00:00 2001 From: demolaf Date: Tue, 21 Jul 2026 12:56:03 +0100 Subject: [PATCH 4/5] fix(firestore): fix PageKey.equals NPE on null snapshots and add hashCode --- .../firebase/ui/firestore/paging/PageKey.java | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/firestore/src/main/java/com/firebase/ui/firestore/paging/PageKey.java b/firestore/src/main/java/com/firebase/ui/firestore/paging/PageKey.java index 45d4e7c7a..db3fc6a9a 100644 --- a/firestore/src/main/java/com/firebase/ui/firestore/paging/PageKey.java +++ b/firestore/src/main/java/com/firebase/ui/firestore/paging/PageKey.java @@ -3,6 +3,8 @@ import com.google.firebase.firestore.DocumentSnapshot; import com.google.firebase.firestore.Query; +import java.util.Objects; + import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.annotation.RestrictTo; @@ -43,18 +45,25 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; PageKey key = (PageKey) o; - if (mStartAfter == null && key.mStartAfter == null && - mEndBefore == null && key.mEndBefore == null) - return true; - return mStartAfter.getId().equals(key.mStartAfter.getId()) && - mEndBefore.getId().equals(key.mEndBefore.getId()); + return Objects.equals(documentId(mStartAfter), documentId(key.mStartAfter)) && + Objects.equals(documentId(mEndBefore), documentId(key.mEndBefore)); + } + + @Override + public int hashCode() { + return Objects.hash(documentId(mStartAfter), documentId(mEndBefore)); + } + + @Nullable + private static String documentId(@Nullable DocumentSnapshot snapshot) { + return snapshot == null ? null : snapshot.getId(); } @Override @NonNull public String toString() { - String startAfter = mStartAfter == null ? null : mStartAfter.getId(); - String endBefore = mEndBefore == null ? null : mEndBefore.getId(); + String startAfter = documentId(mStartAfter); + String endBefore = documentId(mEndBefore); return "PageKey{" + "StartAfter=" + startAfter + ", EndBefore=" + endBefore + From 82ed9ee192a74cb6bd4bc8092c6f8e006ebd8e90 Mon Sep 17 00:00:00 2001 From: demolaf Date: Tue, 21 Jul 2026 12:56:03 +0100 Subject: [PATCH 5/5] test(firestore): run paging source test in CI via robolectric and replace sleeps with latches --- firestore/build.gradle.kts | 9 +++ .../paging/FirestorePagingSourceTest.java | 78 +++++++++++++++---- 2 files changed, 73 insertions(+), 14 deletions(-) diff --git a/firestore/build.gradle.kts b/firestore/build.gradle.kts index 6074dab4d..d048e6c63 100644 --- a/firestore/build.gradle.kts +++ b/firestore/build.gradle.kts @@ -19,6 +19,9 @@ android { testOptions { targetSdk = Config.SdkVersions.target + unitTests { + isIncludeAndroidResources = true + } } lint { @@ -64,6 +67,12 @@ dependencies { lintChecks(project(":lint")) + testImplementation(libs.junit) + testImplementation(libs.robolectric) + testImplementation(libs.test.core) + testImplementation(libs.mockito.core) + testImplementation(libs.androidx.paging) + androidTestImplementation(libs.arch.core.testing) androidTestImplementation(libs.test.core) androidTestImplementation(libs.junit) diff --git a/firestore/src/test/java/com/firebase/ui/firestore/paging/FirestorePagingSourceTest.java b/firestore/src/test/java/com/firebase/ui/firestore/paging/FirestorePagingSourceTest.java index 9ba76d27b..810efb616 100644 --- a/firestore/src/test/java/com/firebase/ui/firestore/paging/FirestorePagingSourceTest.java +++ b/firestore/src/test/java/com/firebase/ui/firestore/paging/FirestorePagingSourceTest.java @@ -1,6 +1,9 @@ package com.firebase.ui.firestore.paging; -import com.google.android.gms.tasks.TaskCompletionSource; +import com.google.android.gms.tasks.OnCanceledListener; +import com.google.android.gms.tasks.OnFailureListener; +import com.google.android.gms.tasks.OnSuccessListener; +import com.google.android.gms.tasks.Task; import com.google.android.gms.tasks.Tasks; import com.google.firebase.firestore.DocumentSnapshot; import com.google.firebase.firestore.Query; @@ -12,28 +15,35 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; import java.util.ArrayList; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executor; +import java.util.concurrent.TimeUnit; import androidx.paging.PagingSource; import androidx.paging.PagingSource.LoadParams.Append; import androidx.paging.PagingSource.LoadParams.Refresh; import androidx.paging.PagingSource.LoadResult.Page; -import androidx.test.ext.junit.runners.AndroidJUnit4; import io.reactivex.rxjava3.disposables.Disposable; import io.reactivex.rxjava3.functions.Consumer; import io.reactivex.rxjava3.plugins.RxJavaPlugins; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -@RunWith(AndroidJUnit4.class) +@RunWith(RobolectricTestRunner.class) +@Config(manifest = Config.NONE) public class FirestorePagingSourceTest { @Mock @@ -110,32 +120,51 @@ public void testLoadAfter_failure() { assertEquals(expected, actual); } + /** + * Cancelling a load interrupts the thread blocked in Tasks.await(). The resulting + * InterruptedException must not escape the callable: by then the subscription is disposed, so + * RxJava would report it as an UndeliverableException and crash the app. + * + * See https://github.com/firebase/FirebaseUI-Android/issues/2008 + */ @Test - public void testLoadSingle_interruptedWhileDisposed_doesNotThrowUndeliverableException() - throws InterruptedException { + public void testLoadSingle_interruptedWhileDisposed_doesNotReportUndeliverableException() + throws Exception { FirestorePagingSource pagingSource = new FirestorePagingSource(mMockQuery, Source.DEFAULT); - TaskCompletionSource taskCompletionSource = new TaskCompletionSource<>(); - when(mMockQuery.get(Source.DEFAULT)).thenReturn(taskCompletionSource.getTask()); + + // Counted down once Tasks.await() starts registering listeners, which means the load is + // genuinely running and about to block. Without waiting on this, the test could dispose + // before the callable ever ran — and would then pass even against the unfixed source. + CountDownLatch loadInFlight = new CountDownLatch(1); + mockQueryNeverCompletes(loadInFlight); List undeliverableErrors = new CopyOnWriteArrayList<>(); + CountDownLatch undeliverableReported = new CountDownLatch(1); Consumer originalErrorHandler = RxJavaPlugins.getErrorHandler(); - RxJavaPlugins.setErrorHandler(undeliverableErrors::add); + RxJavaPlugins.setErrorHandler(throwable -> { + undeliverableErrors.add(throwable); + undeliverableReported.countDown(); + }); + try { Refresh refreshRequest = new Refresh<>(null, 2, false); Disposable disposable = pagingSource.loadSingle(refreshRequest) .subscribe(result -> { }, error -> { }); - Thread.sleep(300); + assertTrue("Load never reached Tasks.await(), so nothing was interrupted", + loadInFlight.await(5, TimeUnit.SECONDS)); + + // Interrupts the worker thread blocked in Tasks.await(). disposable.dispose(); - Thread.sleep(300); - assertTrue( - "Interruption during an in-flight load must not surface as an " - + "undeliverable RxJava exception: " + undeliverableErrors, - undeliverableErrors.isEmpty()); + assertFalse("Interrupting an in-flight load must not surface as an undeliverable " + + "RxJava exception: " + undeliverableErrors, + undeliverableReported.await(2, TimeUnit.SECONDS)); } finally { RxJavaPlugins.setErrorHandler(originalErrorHandler); } + + verify(mMockQuery).get(Source.DEFAULT); } private void initMockQuery() { @@ -154,4 +183,25 @@ private void mockQuerySuccess(List snapshots) { private void mockQueryFailure(Exception exception) { when(mMockQuery.get(Source.DEFAULT)).thenReturn(Tasks.forException(exception)); } + + /** + * Stubs the query with a task that never completes, so Tasks.await() blocks until the thread + * is interrupted. {@code inFlight} is counted down once await() has started. + */ + @SuppressWarnings("unchecked") + private void mockQueryNeverCompletes(CountDownLatch inFlight) { + Task neverCompletes = mock(Task.class); + when(neverCompletes.isComplete()).thenReturn(false); + when(neverCompletes.addOnSuccessListener(any(Executor.class), any(OnSuccessListener.class))) + .thenAnswer(invocation -> { + inFlight.countDown(); + return neverCompletes; + }); + when(neverCompletes.addOnFailureListener(any(Executor.class), any(OnFailureListener.class))) + .thenReturn(neverCompletes); + when(neverCompletes.addOnCanceledListener(any(Executor.class), any(OnCanceledListener.class))) + .thenReturn(neverCompletes); + + when(mMockQuery.get(Source.DEFAULT)).thenReturn(neverCompletes); + } }