From 8d14efef87bd8eafb5a6a66d6dc8139e9efc2fa2 Mon Sep 17 00:00:00 2001 From: Lee Kellogg Date: Tue, 22 Nov 2022 16:14:46 -0500 Subject: [PATCH 01/12] Remove some synchronized blocks from FirebaseAppDistributionImpl --- .../appdistribution/impl/UpdateTaskCache.java | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/UpdateTaskCache.java diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/UpdateTaskCache.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/UpdateTaskCache.java new file mode 100644 index 00000000000..d74bfb38c32 --- /dev/null +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/UpdateTaskCache.java @@ -0,0 +1,47 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.appdistribution.impl; + +import com.google.android.gms.tasks.Task; + +/** + * A cache for Tasks, for use in cases where we only ever want one active task at a time for a + * particular operation. + */ +class TaskCache { + + /** A functional interface for a producer of a new Task. */ + interface TaskProducer { + + /** Produce a new Task. */ + T produce(); + } + + private T cachedTask; + + /** + * Gets a cached task, if there is one and it is not completed, or else calls the given {@code + * producer} and caches the returned task. + * + * @return the cached task if there is one and it is not completed, or else the result from {@code + * producer.produce()} + */ + synchronized T getOrCreateTask(TaskProducer producer) { + if (cachedTask == null || cachedTask.isComplete()) { + cachedTask = producer.produce(); + } + return cachedTask; + } +} From 218af55ca4dc99f303a6d2906e98c3975a1b1197 Mon Sep 17 00:00:00 2001 From: Lee Kellogg Date: Tue, 22 Nov 2022 16:50:50 -0500 Subject: [PATCH 02/12] Rename UpdateTaskCache.java to TaskCache.java --- .../appdistribution/impl/UpdateTaskCache.java | 47 ------------------- 1 file changed, 47 deletions(-) delete mode 100644 firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/UpdateTaskCache.java diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/UpdateTaskCache.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/UpdateTaskCache.java deleted file mode 100644 index d74bfb38c32..00000000000 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/UpdateTaskCache.java +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright 2022 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.firebase.appdistribution.impl; - -import com.google.android.gms.tasks.Task; - -/** - * A cache for Tasks, for use in cases where we only ever want one active task at a time for a - * particular operation. - */ -class TaskCache { - - /** A functional interface for a producer of a new Task. */ - interface TaskProducer { - - /** Produce a new Task. */ - T produce(); - } - - private T cachedTask; - - /** - * Gets a cached task, if there is one and it is not completed, or else calls the given {@code - * producer} and caches the returned task. - * - * @return the cached task if there is one and it is not completed, or else the result from {@code - * producer.produce()} - */ - synchronized T getOrCreateTask(TaskProducer producer) { - if (cachedTask == null || cachedTask.isComplete()) { - cachedTask = producer.produce(); - } - return cachedTask; - } -} From 2f398e62f3ab5012caf74c46118fcb2a4b7214b1 Mon Sep 17 00:00:00 2001 From: Lee Kellogg Date: Tue, 22 Nov 2022 16:49:39 -0500 Subject: [PATCH 03/12] Remove more sync blocks around new release cache --- .../impl/FirebaseAppDistributionImpl.java | 103 ++++++++-------- .../FirebaseAppDistributionRegistrar.java | 14 ++- .../impl/SequentialReference.java | 87 +++++++++++++ ...irebaseAppDistributionServiceImplTest.java | 115 ++++++++++-------- 4 files changed, 208 insertions(+), 111 deletions(-) create mode 100644 firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java index 5b7be60f5cb..51fb55b96cc 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java @@ -25,7 +25,6 @@ import android.app.Activity; import android.app.AlertDialog; import android.content.Context; -import androidx.annotation.GuardedBy; import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; @@ -41,6 +40,7 @@ import com.google.firebase.appdistribution.UpdateProgress; import com.google.firebase.appdistribution.UpdateStatus; import com.google.firebase.appdistribution.UpdateTask; +import java.util.concurrent.Executor; /** * This class is the "real" implementation of the Firebase App Distribution API which should only be @@ -57,14 +57,10 @@ class FirebaseAppDistributionImpl implements FirebaseAppDistribution { private final ApkUpdater apkUpdater; private final AabUpdater aabUpdater; private final SignInStorage signInStorage; - - private final Object cachedNewReleaseLock = new Object(); - - @GuardedBy("cachedNewReleaseLock") - private AppDistributionReleaseInternal cachedNewRelease; - + private final SequentialReference cachedNewRelease; private TaskCache updateIfNewReleaseAvailableTaskCache = new TaskCache<>(); private TaskCache> checkForNewReleaseTaskCache = new TaskCache<>(); + private Executor lightweightExecutor; private AlertDialog updateConfirmationDialog; private AlertDialog signInConfirmationDialog; @Nullable private Activity dialogHostActivity = null; @@ -83,7 +79,8 @@ class FirebaseAppDistributionImpl implements FirebaseAppDistribution { @NonNull ApkUpdater apkUpdater, @NonNull AabUpdater aabUpdater, @NonNull SignInStorage signInStorage, - @NonNull FirebaseAppDistributionLifecycleNotifier lifecycleNotifier) { + @NonNull FirebaseAppDistributionLifecycleNotifier lifecycleNotifier, + @NonNull Executor lightweightExecutor) { this.firebaseApp = firebaseApp; this.testerSignInManager = testerSignInManager; this.newReleaseFetcher = newReleaseFetcher; @@ -91,6 +88,8 @@ class FirebaseAppDistributionImpl implements FirebaseAppDistribution { this.aabUpdater = aabUpdater; this.signInStorage = signInStorage; this.lifecycleNotifier = lifecycleNotifier; + this.cachedNewRelease = new SequentialReference<>(lightweightExecutor); + this.lightweightExecutor = lightweightExecutor; lifecycleNotifier.addOnActivityDestroyedListener(this::onActivityDestroyed); lifecycleNotifier.addOnActivityPausedListener(this::onActivityPaused); lifecycleNotifier.addOnActivityResumedListener(this::onActivityResumed); @@ -110,9 +109,10 @@ public UpdateTask updateIfNewReleaseAvailable() { lifecycleNotifier .applyToForegroundActivityTask(this::showSignInConfirmationDialog) - .onSuccessTask(unused -> signInTester()) - .onSuccessTask(unused -> checkForNewRelease()) + .onSuccessTask(lightweightExecutor, unused -> signInTester()) + .onSuccessTask(lightweightExecutor, unused -> checkForNewRelease()) .continueWithTask( + lightweightExecutor, task -> { if (!task.isSuccessful()) { postProgressToCachedUpdateIfNewReleaseTask( @@ -141,13 +141,16 @@ public UpdateTask updateIfNewReleaseAvailable() { activity -> showUpdateConfirmationDialog(activity, release)); }) .onSuccessTask( + lightweightExecutor, unused -> updateApp(true) .addOnProgressListener( + lightweightExecutor, updateProgress -> postProgressToCachedUpdateIfNewReleaseTask( updateTask, updateProgress))) .addOnFailureListener( + lightweightExecutor, exception -> setCachedUpdateIfNewReleaseCompletionError(updateTask, exception)); return updateTask; @@ -214,7 +217,7 @@ public Task signInTester() { @Override public void signOutTester() { - setCachedNewRelease(null); + cachedNewRelease.set(null); signInStorage.setSignInStatus(false); } @@ -234,13 +237,15 @@ public synchronized Task checkForNewRelease() { return newReleaseFetcher .checkForNewRelease() .onSuccessTask( + lightweightExecutor, appDistributionReleaseInternal -> { - setCachedNewRelease(appDistributionReleaseInternal); + cachedNewRelease.set(appDistributionReleaseInternal); return Tasks.forResult( ReleaseUtils.convertToAppDistributionRelease( appDistributionReleaseInternal)); }) .addOnFailureListener( + lightweightExecutor, e -> { if (e instanceof FirebaseAppDistributionException && ((FirebaseAppDistributionException) e).getErrorCode() @@ -265,34 +270,35 @@ public UpdateTask updateApp() { * basic configuration and false for advanced configuration. */ private UpdateTask updateApp(boolean showDownloadInNotificationManager) { - synchronized (cachedNewReleaseLock) { - if (!isTesterSignedIn()) { - UpdateTaskImpl updateTask = new UpdateTaskImpl(); - updateTask.setException( - new FirebaseAppDistributionException( - "Tester is not signed in", AUTHENTICATION_FAILURE)); - return updateTask; - } - if (cachedNewRelease == null) { - LogWrapper.getInstance().v("New release not found."); - return getErrorUpdateTask( - new FirebaseAppDistributionException( - ErrorMessages.RELEASE_NOT_FOUND_ERROR, UPDATE_NOT_AVAILABLE)); - } - if (cachedNewRelease.getDownloadUrl() == null) { - LogWrapper.getInstance().v("Download failed to execute."); - return getErrorUpdateTask( - new FirebaseAppDistributionException( - ErrorMessages.DOWNLOAD_URL_NOT_FOUND, - FirebaseAppDistributionException.Status.DOWNLOAD_FAILURE)); - } + return cachedNewRelease.applyUpdateTask( + release -> { + if (!isTesterSignedIn()) { + UpdateTaskImpl updateTask = new UpdateTaskImpl(); + updateTask.setException( + new FirebaseAppDistributionException( + "Tester is not signed in", AUTHENTICATION_FAILURE)); + return updateTask; + } + if (release == null) { + LogWrapper.getInstance().v("New release not found."); + return getErrorUpdateTask( + new FirebaseAppDistributionException( + ErrorMessages.RELEASE_NOT_FOUND_ERROR, UPDATE_NOT_AVAILABLE)); + } + if (release.getDownloadUrl() == null) { + LogWrapper.getInstance().v("Download failed to execute."); + return getErrorUpdateTask( + new FirebaseAppDistributionException( + ErrorMessages.DOWNLOAD_URL_NOT_FOUND, + FirebaseAppDistributionException.Status.DOWNLOAD_FAILURE)); + } - if (cachedNewRelease.getBinaryType() == BinaryType.AAB) { - return aabUpdater.updateAab(cachedNewRelease); - } else { - return apkUpdater.updateApk(cachedNewRelease, showDownloadInNotificationManager); - } - } + if (release.getBinaryType() == BinaryType.AAB) { + return aabUpdater.updateAab(release); + } else { + return apkUpdater.updateApk(release, showDownloadInNotificationManager); + } + }); } @VisibleForTesting @@ -313,10 +319,8 @@ void onActivityResumed(Activity activity) { new FirebaseAppDistributionException( ErrorMessages.HOST_ACTIVITY_INTERRUPTED, HOST_ACTIVITY_INTERRUPTED)); } else { - synchronized (cachedNewReleaseLock) { - showUpdateConfirmationDialog( - activity, ReleaseUtils.convertToAppDistributionRelease(cachedNewRelease)); - } + showUpdateConfirmationDialog( + activity, ReleaseUtils.convertToAppDistributionRelease(cachedNewRelease.get())); } } } @@ -342,17 +346,8 @@ void onActivityDestroyed(@NonNull Activity activity) { } @VisibleForTesting - void setCachedNewRelease(@Nullable AppDistributionReleaseInternal newRelease) { - synchronized (cachedNewReleaseLock) { - cachedNewRelease = newRelease; - } - } - - @VisibleForTesting - AppDistributionReleaseInternal getCachedNewRelease() { - synchronized (cachedNewReleaseLock) { - return cachedNewRelease; - } + SequentialReference getCachedNewRelease() { + return cachedNewRelease; } private Task showUpdateConfirmationDialog( diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionRegistrar.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionRegistrar.java index 79e4ecc491c..c11d2f98d28 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionRegistrar.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionRegistrar.java @@ -20,6 +20,7 @@ import androidx.annotation.NonNull; import com.google.firebase.FirebaseApp; import com.google.firebase.annotations.concurrent.Blocking; +import com.google.firebase.annotations.concurrent.Lightweight; import com.google.firebase.appdistribution.FirebaseAppDistribution; import com.google.firebase.components.Component; import com.google.firebase.components.ComponentContainer; @@ -47,13 +48,19 @@ public class FirebaseAppDistributionRegistrar implements ComponentRegistrar { @Override public @NonNull List> getComponents() { Qualified blockingExecutor = Qualified.qualified(Blocking.class, Executor.class); + Qualified lightweightExecutor = + Qualified.qualified(Lightweight.class, Executor.class); return Arrays.asList( Component.builder(FirebaseAppDistribution.class) .name(LIBRARY_NAME) .add(Dependency.required(FirebaseApp.class)) .add(Dependency.requiredProvider(FirebaseInstallationsApi.class)) .add(Dependency.required(blockingExecutor)) - .factory(c -> buildFirebaseAppDistribution(c, c.get(blockingExecutor))) + .add(Dependency.required(lightweightExecutor)) + .factory( + c -> + buildFirebaseAppDistribution( + c, c.get(blockingExecutor), c.get(lightweightExecutor))) // construct FirebaseAppDistribution instance on startup so we can register for // activity lifecycle callbacks before the API is called .alwaysEager() @@ -62,7 +69,7 @@ public class FirebaseAppDistributionRegistrar implements ComponentRegistrar { } private FirebaseAppDistribution buildFirebaseAppDistribution( - ComponentContainer container, Executor blockingExecutor) { + ComponentContainer container, Executor blockingExecutor, Executor lightweightExecutor) { FirebaseApp firebaseApp = container.get(FirebaseApp.class); Context context = firebaseApp.getApplicationContext(); Provider firebaseInstallationsApiProvider = @@ -86,7 +93,8 @@ private FirebaseAppDistribution buildFirebaseAppDistribution( new ApkUpdater(firebaseApp, new ApkInstaller(), blockingExecutor), new AabUpdater(blockingExecutor), signInStorage, - lifecycleNotifier); + lifecycleNotifier, + lightweightExecutor); if (context instanceof Application) { Application firebaseApplication = (Application) context; diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java new file mode 100644 index 00000000000..677dd4bc223 --- /dev/null +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java @@ -0,0 +1,87 @@ +package com.google.firebase.appdistribution.impl; + +import com.google.android.gms.tasks.Task; +import com.google.android.gms.tasks.TaskCompletionSource; +import com.google.firebase.appdistribution.UpdateTask; +import com.google.firebase.concurrent.FirebaseExecutors; +import java.util.concurrent.Executor; + +class SequentialReference { + + interface SequentialReferenceConsumer { + void consume(T value); + } + + interface SequentialReferenceTransformer { + T transform(T value); + } + + interface SequentialReferenceTaskTransformer { + Task transform(T value); + } + + interface SequentialReferenceUpdateTaskTransformer { + UpdateTask transform(T value); + } + + private final Executor sequentialExecutor; + + private T value; + + SequentialReference(Executor baseExecutor) { + this(baseExecutor, null); + } + + SequentialReference(Executor baseExecutor, T initialValue) { + sequentialExecutor = FirebaseExecutors.newSequentialExecutor(baseExecutor); + value = initialValue; + } + + T get() { + return value; + } + + void set(T newValue) { + sequentialExecutor.execute( + () -> { + value = newValue; + }); + } + + void consume(SequentialReferenceConsumer listener) { + sequentialExecutor.execute( + () -> { + listener.consume(value); + }); + } + + void transform(SequentialReferenceTransformer transformer) { + sequentialExecutor.execute( + () -> { + value = transformer.transform(value); + }); + } + + Task applyTask(SequentialReferenceTaskTransformer transformer) { + TaskCompletionSource taskCompletionSource = new TaskCompletionSource<>(); + sequentialExecutor.execute( + () -> + transformer + .transform(value) + .addOnSuccessListener(sequentialExecutor, taskCompletionSource::setResult) + .addOnFailureListener(sequentialExecutor, taskCompletionSource::setException)); + return taskCompletionSource.getTask(); + } + + UpdateTask applyUpdateTask(SequentialReferenceUpdateTaskTransformer transformer) { + UpdateTaskImpl updateTask = new UpdateTaskImpl(); + sequentialExecutor.execute( + () -> + transformer + .transform(value) + .addOnProgressListener(sequentialExecutor, updateTask::updateProgress) + .addOnSuccessListener(sequentialExecutor, result -> updateTask.setResult()) + .addOnFailureListener(sequentialExecutor, updateTask::setException)); + return updateTask; + } +} diff --git a/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java b/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java index 63af4c0a2a1..84fa1f67127 100644 --- a/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java +++ b/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java @@ -65,6 +65,7 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executor; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; @@ -114,6 +115,8 @@ public class FirebaseAppDistributionServiceImplTest { .setBinaryType(BinaryType.APK) .setDownloadUrl(TEST_URL); + private final ExecutorService testExecutor = Executors.newSingleThreadExecutor(); + private FirebaseAppDistributionImpl firebaseAppDistribution; private TestActivity activity; private FirebaseApp firebaseApp; @@ -154,7 +157,8 @@ public void setup() throws FirebaseAppDistributionException { mockApkUpdater, mockAabUpdater, mockSignInStorage, - mockLifecycleNotifier)); + mockLifecycleNotifier, + testExecutor)); when(mockTesterSignInManager.signInTester()).thenReturn(Tasks.forResult(null)); when(mockSignInStorage.getSignInStatus()).thenReturn(true); @@ -183,7 +187,7 @@ public void setup() throws FirebaseAppDistributionException { } @Test - public void checkForNewRelease_whenCheckForNewReleaseFails_throwsError() { + public void checkForNewRelease_whenCheckForNewReleaseFails_throwsError() throws InterruptedException { when(mockNewReleaseFetcher.checkForNewRelease()) .thenReturn( Tasks.forException( @@ -191,67 +195,68 @@ public void checkForNewRelease_whenCheckForNewReleaseFails_throwsError() { ErrorMessages.JSON_PARSING_ERROR, Status.NETWORK_FAILURE))); Task task = firebaseAppDistribution.checkForNewRelease(); - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); assertTaskFailure(task, NETWORK_FAILURE, JSON_PARSING_ERROR); } @Test - public void checkForNewRelease_testerIsNotSignedIn_taskFails() { + public void checkForNewRelease_testerIsNotSignedIn_taskFails() throws InterruptedException { when(firebaseAppDistribution.isTesterSignedIn()).thenReturn(false); Task task = firebaseAppDistribution.checkForNewRelease(); - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); assertTaskFailure(task, AUTHENTICATION_FAILURE, "Tester is not signed in"); } @Test - public void checkForNewRelease_whenCheckForNewReleaseSucceeds_returnsRelease() { + public void checkForNewRelease_whenCheckForNewReleaseSucceeds_returnsRelease() throws InterruptedException { when(mockNewReleaseFetcher.checkForNewRelease()) .thenReturn( Tasks.forResult( TEST_RELEASE_NEWER_AAB_INTERNAL.setReleaseNotes("Newer version.").build())); Task task = firebaseAppDistribution.checkForNewRelease(); - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); assertNotNull(task.getResult()); assertEquals(TEST_RELEASE_NEWER_AAB, task.getResult()); assertEquals( - TEST_RELEASE_NEWER_AAB_INTERNAL.build(), firebaseAppDistribution.getCachedNewRelease()); + TEST_RELEASE_NEWER_AAB_INTERNAL.build(), + firebaseAppDistribution.getCachedNewRelease().get()); } @Test - public void checkForNewRelease_authenticationFailure_signOutTester() { + public void checkForNewRelease_authenticationFailure_signOutTester() throws InterruptedException { when(mockNewReleaseFetcher.checkForNewRelease()) .thenReturn( Tasks.forException( new FirebaseAppDistributionException("Test", AUTHENTICATION_FAILURE))); firebaseAppDistribution.checkForNewRelease(); - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); verify(mockSignInStorage, times(1)).setSignInStatus(false); } @Test - public void updateApp_whenNotSignedIn_throwsError() { + public void updateApp_whenNotSignedIn_throwsError() throws InterruptedException { when(mockSignInStorage.getSignInStatus()).thenReturn(false); UpdateTask updateTask = firebaseAppDistribution.updateApp(); - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); assertTaskFailure(updateTask, AUTHENTICATION_FAILURE, "Tester is not signed in"); } @Test - public void updateIfNewReleaseAvailable_whenNewAabReleaseAvailable_showsUpdateDialog() { + public void updateIfNewReleaseAvailable_whenNewAabReleaseAvailable_showsUpdateDialog() throws InterruptedException { when(mockNewReleaseFetcher.checkForNewRelease()) .thenReturn(Tasks.forResult((TEST_RELEASE_NEWER_AAB_INTERNAL.build()))); firebaseAppDistribution.updateIfNewReleaseAvailable(); - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); AlertDialog dialog = assertAlertDialogShown(); assertEquals( @@ -277,12 +282,12 @@ public void updateIfNewReleaseAvailable_fromABackgroundThread_showsUpdateDialog( } @Test - public void updateIfNewReleaseAvailable_whenReleaseNotesEmpty_doesNotShowReleaseNotes() { + public void updateIfNewReleaseAvailable_whenReleaseNotesEmpty_doesNotShowReleaseNotes() throws InterruptedException { when(mockNewReleaseFetcher.checkForNewRelease()) .thenReturn(Tasks.forResult((TEST_RELEASE_NEWER_AAB_INTERNAL.setReleaseNotes("").build()))); firebaseAppDistribution.updateIfNewReleaseAvailable(); - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); AlertDialog dialog = assertAlertDialogShown(); assertEquals( @@ -293,14 +298,14 @@ public void updateIfNewReleaseAvailable_whenReleaseNotesEmpty_doesNotShowRelease } @Test - public void updateIfNewReleaseAvailable_whenNoReleaseAvailable_updateDialogNotShown() { + public void updateIfNewReleaseAvailable_whenNoReleaseAvailable_updateDialogNotShown() throws InterruptedException { when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(null)); UpdateTask task = firebaseAppDistribution.updateIfNewReleaseAvailable(); List progressEvents = new ArrayList<>(); task.addOnProgressListener(progressEvents::add); - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); assertEquals(1, progressEvents.size()); assertEquals(UpdateStatus.NEW_RELEASE_NOT_AVAILABLE, progressEvents.get(0).getUpdateStatus()); @@ -308,7 +313,7 @@ public void updateIfNewReleaseAvailable_whenNoReleaseAvailable_updateDialogNotSh } @Test - public void updateIfNewReleaseAvailable_whenActivityBackgrounded_updateDialogNotShown() { + public void updateIfNewReleaseAvailable_whenActivityBackgrounded_updateDialogNotShown() throws InterruptedException { when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(null)); UpdateTask task = firebaseAppDistribution.updateIfNewReleaseAvailable(); @@ -316,14 +321,14 @@ public void updateIfNewReleaseAvailable_whenActivityBackgrounded_updateDialogNot List progressEvents = new ArrayList<>(); task.addOnProgressListener(progressEvents::add); - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); assertEquals(1, progressEvents.size()); assertEquals(UpdateStatus.NEW_RELEASE_NOT_AVAILABLE, progressEvents.get(0).getUpdateStatus()); assertNull(ShadowAlertDialog.getLatestAlertDialog()); } @Test - public void updateIfNewReleaseAvailable_whenSignInCancelled_checkForUpdateNotCalled() { + public void updateIfNewReleaseAvailable_whenSignInCancelled_checkForUpdateNotCalled() throws InterruptedException { when(mockSignInStorage.getSignInStatus()).thenReturn(false); when(mockTesterSignInManager.signInTester()) .thenReturn( @@ -336,14 +341,14 @@ public void updateIfNewReleaseAvailable_whenSignInCancelled_checkForUpdateNotCal AlertDialog signInDialog = assertAlertDialogShown(); signInDialog.getButton(DialogInterface.BUTTON_POSITIVE).performClick(); - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); verify(mockTesterSignInManager, times(1)).signInTester(); verify(mockNewReleaseFetcher, never()).checkForNewRelease(); assertTaskFailure(updateTask, AUTHENTICATION_CANCELED, ErrorMessages.AUTHENTICATION_CANCELED); } @Test - public void updateIfNewReleaseAvailable_whenSignInFailed_checkForUpdateNotCalled() { + public void updateIfNewReleaseAvailable_whenSignInFailed_checkForUpdateNotCalled() throws InterruptedException { when(mockSignInStorage.getSignInStatus()).thenReturn(false); when(mockTesterSignInManager.signInTester()) .thenReturn( @@ -355,22 +360,22 @@ public void updateIfNewReleaseAvailable_whenSignInFailed_checkForUpdateNotCalled AlertDialog signInDialog = assertAlertDialogShown(); signInDialog.getButton(DialogInterface.BUTTON_POSITIVE).performClick(); - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); assertTaskFailure(updateTask, AUTHENTICATION_FAILURE, AUTHENTICATION_ERROR); } @Test - public void updateIfNewReleaseAvailable_whenDialogDismissed_taskFails() { + public void updateIfNewReleaseAvailable_whenDialogDismissed_taskFails() throws InterruptedException { when(mockNewReleaseFetcher.checkForNewRelease()) .thenReturn(Tasks.forResult(TEST_RELEASE_NEWER_AAB_INTERNAL.build())); UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable(); - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); AlertDialog updateDialog = assertAlertDialogShown(); updateDialog.getButton(AlertDialog.BUTTON_NEGATIVE).performClick(); // dismiss dialog - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); assertFalse(updateDialog.isShowing()); assertFalse(updateTask.isSuccessful()); @@ -378,16 +383,16 @@ public void updateIfNewReleaseAvailable_whenDialogDismissed_taskFails() { } @Test - public void updateIfNewReleaseAvailable_whenDialogCanceled_taskFails() { + public void updateIfNewReleaseAvailable_whenDialogCanceled_taskFails() throws InterruptedException { when(mockNewReleaseFetcher.checkForNewRelease()) .thenReturn(Tasks.forResult(TEST_RELEASE_NEWER_AAB_INTERNAL.build())); UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable(); - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); AlertDialog updateDialog = assertAlertDialogShown(); updateDialog.onBackPressed(); // cancels the dialog - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); assertFalse(updateDialog.isShowing()); @@ -395,7 +400,7 @@ public void updateIfNewReleaseAvailable_whenDialogCanceled_taskFails() { } @Test - public void updateIfNewReleaseAvailable_whenCheckForUpdateFails_updateAppNotCalled() { + public void updateIfNewReleaseAvailable_whenCheckForUpdateFails_updateAppNotCalled() throws InterruptedException { when(mockNewReleaseFetcher.checkForNewRelease()) .thenReturn( Tasks.forException( @@ -407,7 +412,7 @@ public void updateIfNewReleaseAvailable_whenCheckForUpdateFails_updateAppNotCall List progressEvents = new ArrayList<>(); updateTask.addOnProgressListener(progressEvents::add); - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); assertEquals(1, progressEvents.size()); assertEquals(UpdateStatus.NEW_RELEASE_CHECK_FAILED, progressEvents.get(0).getUpdateStatus()); @@ -416,23 +421,23 @@ public void updateIfNewReleaseAvailable_whenCheckForUpdateFails_updateAppNotCall } @Test - public void updateIfNewReleaseAvailable_whenTesterIsSignedIn_doesNotOpenDialog() { + public void updateIfNewReleaseAvailable_whenTesterIsSignedIn_doesNotOpenDialog() throws InterruptedException { when(mockSignInStorage.getSignInStatus()).thenReturn(true); firebaseAppDistribution.updateIfNewReleaseAvailable(); - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); assertNull(ShadowAlertDialog.getLatestAlertDialog()); } @Test - public void signInTester_whenDialogDismissed_taskFails() { + public void signInTester_whenDialogDismissed_taskFails() throws InterruptedException { when(mockSignInStorage.getSignInStatus()).thenReturn(false); Task updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable(); AlertDialog dialog = assertAlertDialogShown(); dialog.getButton(AlertDialog.BUTTON_NEGATIVE).performClick(); // dismiss dialog - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); assertFalse(updateTask.isSuccessful()); Exception e = updateTask.getException(); @@ -442,13 +447,13 @@ public void signInTester_whenDialogDismissed_taskFails() { } @Test - public void updateIfNewReleaseAvailable_whenSignInDialogCanceled_taskFails() { + public void updateIfNewReleaseAvailable_whenSignInDialogCanceled_taskFails() throws InterruptedException { when(mockSignInStorage.getSignInStatus()).thenReturn(false); Task signInTask = firebaseAppDistribution.updateIfNewReleaseAvailable(); AlertDialog dialog = assertAlertDialogShown(); dialog.onBackPressed(); // cancel dialog - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); assertFalse(signInTask.isSuccessful()); Exception e = signInTask.getException(); @@ -472,7 +477,7 @@ public void signOutTester_setsSignInStatusFalse() { } @Test - public void updateIfNewReleaseAvailable_receiveProgressUpdateFromUpdateApp() { + public void updateIfNewReleaseAvailable_receiveProgressUpdateFromUpdateApp() throws InterruptedException { AppDistributionReleaseInternal newRelease = TEST_RELEASE_NEWER_AAB_INTERNAL.build(); when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(newRelease)); UpdateTaskImpl updateTaskToReturn = new UpdateTaskImpl(); @@ -488,12 +493,13 @@ public void updateIfNewReleaseAvailable_receiveProgressUpdateFromUpdateApp() { List progressEvents = new ArrayList<>(); updateTask.addOnProgressListener(progressEvents::add); - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); // Clicking the update button. AlertDialog updateDialog = (AlertDialog) ShadowAlertDialog.getLatestDialog(); updateDialog.getButton(Dialog.BUTTON_POSITIVE).performClick(); - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); +// TestUtils.awaitAsyncOperations(testExecutor); // Update flow assertEquals(1, progressEvents.size()); @@ -515,12 +521,12 @@ public void updateIfNewReleaseAvailable_fromABackgroundThread_showsSignInDialog( } @Test - public void updateIfNewReleaseAvailable_whenScreenRotates_signInConfirmationDialogReappears() { + public void updateIfNewReleaseAvailable_whenScreenRotates_signInConfirmationDialogReappears() throws InterruptedException { when(mockSignInStorage.getSignInStatus()).thenReturn(false); when(activity.isChangingConfigurations()).thenReturn(true); UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable(); - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); // Mimic activity recreation due to a configuration change firebaseAppDistribution.onActivityDestroyed(activity); @@ -531,13 +537,13 @@ public void updateIfNewReleaseAvailable_whenScreenRotates_signInConfirmationDial } @Test - public void updateIfNewReleaseAvailable_whenScreenRotates_updateDialogReappears() { + public void updateIfNewReleaseAvailable_whenScreenRotates_updateDialogReappears() throws InterruptedException { AppDistributionReleaseInternal newRelease = TEST_RELEASE_NEWER_AAB_INTERNAL.build(); when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(newRelease)); when(activity.isChangingConfigurations()).thenReturn(true); UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable(); - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); // Mimic activity recreation due to a configuration change firebaseAppDistribution.onActivityDestroyed(activity); @@ -549,7 +555,7 @@ public void updateIfNewReleaseAvailable_whenScreenRotates_updateDialogReappears( @Test public void - updateIfNewReleaseAvailable_whenSignInDialogShowingAndNewActivityStarts_signInTaskCancelled() { + updateIfNewReleaseAvailable_whenSignInDialogShowingAndNewActivityStarts_signInTaskCancelled() throws InterruptedException { TestActivity testActivity2 = new TestActivity(); when(mockSignInStorage.getSignInStatus()).thenReturn(false); @@ -559,36 +565,37 @@ public void updateIfNewReleaseAvailable_whenScreenRotates_updateDialogReappears( firebaseAppDistribution.onActivityPaused(activity); firebaseAppDistribution.onActivityResumed(testActivity2); - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); assertTaskFailure( updateTask, HOST_ACTIVITY_INTERRUPTED, ErrorMessages.HOST_ACTIVITY_INTERRUPTED); } @Test public void - updateIfNewReleaseAvailable_whenUpdateDialogShowingAndNewActivityStarts_updateTaskCancelled() { + updateIfNewReleaseAvailable_whenUpdateDialogShowingAndNewActivityStarts_updateTaskCancelled() throws InterruptedException { TestActivity testActivity2 = new TestActivity(); AppDistributionReleaseInternal newRelease = TEST_RELEASE_NEWER_AAB_INTERNAL.build(); when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(newRelease)); UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable(); - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); // Mimic different activity getting resumed firebaseAppDistribution.onActivityPaused(activity); firebaseAppDistribution.onActivityResumed(testActivity2); - shadowOf(getMainLooper()).idle(); + TestUtils.awaitAsyncOperations(testExecutor); assertTaskFailure( updateTask, HOST_ACTIVITY_INTERRUPTED, ErrorMessages.HOST_ACTIVITY_INTERRUPTED); } @Test - public void updateAppTask_whenNoReleaseAvailable_throwsError() { - firebaseAppDistribution.setCachedNewRelease(null); + public void updateAppTask_whenNoReleaseAvailable_throwsError() throws InterruptedException { + firebaseAppDistribution.getCachedNewRelease().set(null); when(mockSignInStorage.getSignInStatus()).thenReturn(true); UpdateTask updateTask = firebaseAppDistribution.updateApp(); + TestUtils.awaitAsyncOperations(testExecutor); assertTaskFailure(updateTask, UPDATE_NOT_AVAILABLE, RELEASE_NOT_FOUND_ERROR); } @@ -596,7 +603,7 @@ public void updateAppTask_whenNoReleaseAvailable_throwsError() { @Test public void updateApp_withAabReleaseAvailable_returnsSameAabTask() { AppDistributionReleaseInternal release = TEST_RELEASE_NEWER_AAB_INTERNAL.build(); - firebaseAppDistribution.setCachedNewRelease(release); + firebaseAppDistribution.getCachedNewRelease().set(release); UpdateTaskImpl updateTaskToReturn = new UpdateTaskImpl(); doReturn(updateTaskToReturn).when(mockAabUpdater).updateAab(release); when(mockSignInStorage.getSignInStatus()).thenReturn(true); @@ -610,7 +617,7 @@ public void updateApp_withAabReleaseAvailable_returnsSameAabTask() { public void updateApp_withApkReleaseAvailable_returnsSameApkTask() { when(mockSignInStorage.getSignInStatus()).thenReturn(true); AppDistributionReleaseInternal release = TEST_RELEASE_NEWER_APK_INTERNAL.build(); - firebaseAppDistribution.setCachedNewRelease(release); + firebaseAppDistribution.getCachedNewRelease().set(release); UpdateTaskImpl updateTaskToReturn = new UpdateTaskImpl(); doReturn(updateTaskToReturn).when(mockApkUpdater).updateApk(release, false); From 4c9243e02e19d923cf2e0710c2cc3abeb5e3ac75 Mon Sep 17 00:00:00 2001 From: Lee Kellogg Date: Wed, 7 Dec 2022 14:39:26 -0500 Subject: [PATCH 04/12] Some refactoring --- .../impl/AppDistributionReleaseInternal.java | 2 + .../impl/FirebaseAppDistributionImpl.java | 85 ++++++++++--------- .../impl/SequentialReference.java | 59 +++++-------- .../appdistribution/impl/UpdateTaskImpl.java | 8 ++ 4 files changed, 74 insertions(+), 80 deletions(-) diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/AppDistributionReleaseInternal.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/AppDistributionReleaseInternal.java index b01a66c6518..ebababf1148 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/AppDistributionReleaseInternal.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/AppDistributionReleaseInternal.java @@ -21,6 +21,8 @@ /** * This class represents the AppDistributionRelease object returned by the App Distribution backend. + * + * TODO(lkellogg): Combine this with AppDistributionReleaseImpl */ @AutoValue abstract class AppDistributionReleaseInternal { diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java index 51fb55b96cc..ce57a395d3b 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java @@ -217,8 +217,10 @@ public Task signInTester() { @Override public void signOutTester() { - cachedNewRelease.set(null); - signInStorage.setSignInStatus(false); + cachedNewRelease.set(() -> { + signInStorage.setSignInStatus(false); + return null; + }); } @Override @@ -226,37 +228,34 @@ public void signOutTester() { // TODO(b/261014422): Use an explicit executor in continuations. @SuppressLint("TaskMainThread") public synchronized Task checkForNewRelease() { - return checkForNewReleaseTaskCache.getOrCreateTask( - () -> { - if (!isTesterSignedIn()) { - return Tasks.forException( - new FirebaseAppDistributionException( - "Tester is not signed in", AUTHENTICATION_FAILURE)); - } + if (!isTesterSignedIn()) { + return Tasks.forException( + new FirebaseAppDistributionException( + "Tester is not signed in", AUTHENTICATION_FAILURE)); + } - return newReleaseFetcher - .checkForNewRelease() - .onSuccessTask( - lightweightExecutor, - appDistributionReleaseInternal -> { - cachedNewRelease.set(appDistributionReleaseInternal); - return Tasks.forResult( - ReleaseUtils.convertToAppDistributionRelease( - appDistributionReleaseInternal)); - }) - .addOnFailureListener( - lightweightExecutor, - e -> { - if (e instanceof FirebaseAppDistributionException - && ((FirebaseAppDistributionException) e).getErrorCode() - == AUTHENTICATION_FAILURE) { - // If CheckForNewRelease returns authentication error, the FID is no longer - // valid or does not have access to the latest release. So sign out the tester - // to force FID re-registration - signOutTester(); - } - }); - }); + return checkForNewReleaseTaskCache.getOrCreateTask( + () -> + newReleaseFetcher + .checkForNewRelease() + .onSuccessTask( + lightweightExecutor, + release -> + cachedNewRelease.setAndTransform( + () -> release, + ReleaseUtils::convertToAppDistributionRelease)) + .addOnFailureListener( + lightweightExecutor, + e -> { + if (e instanceof FirebaseAppDistributionException + && ((FirebaseAppDistributionException) e).getErrorCode() + == AUTHENTICATION_FAILURE) { + // If CheckForNewRelease returns authentication error, the FID is no longer + // valid or does not have access to the latest release. So sign out the tester + // to force FID re-registration + signOutTester(); + } + })); } @Override @@ -270,15 +269,15 @@ public UpdateTask updateApp() { * basic configuration and false for advanced configuration. */ private UpdateTask updateApp(boolean showDownloadInNotificationManager) { - return cachedNewRelease.applyUpdateTask( + if (!isTesterSignedIn()) { + UpdateTaskImpl updateTask = new UpdateTaskImpl(); + updateTask.setException( + new FirebaseAppDistributionException( + "Tester is not signed in", AUTHENTICATION_FAILURE)); + return updateTask; + } + return cachedNewRelease.getAndTransform( release -> { - if (!isTesterSignedIn()) { - UpdateTaskImpl updateTask = new UpdateTaskImpl(); - updateTask.setException( - new FirebaseAppDistributionException( - "Tester is not signed in", AUTHENTICATION_FAILURE)); - return updateTask; - } if (release == null) { LogWrapper.getInstance().v("New release not found."); return getErrorUpdateTask( @@ -319,8 +318,10 @@ void onActivityResumed(Activity activity) { new FirebaseAppDistributionException( ErrorMessages.HOST_ACTIVITY_INTERRUPTED, HOST_ACTIVITY_INTERRUPTED)); } else { - showUpdateConfirmationDialog( - activity, ReleaseUtils.convertToAppDistributionRelease(cachedNewRelease.get())); + cachedNewRelease.get( + release -> + showUpdateConfirmationDialog( + activity, ReleaseUtils.convertToAppDistributionRelease(release))); } } } diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java index 677dd4bc223..d64885cc8d2 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java @@ -1,5 +1,6 @@ package com.google.firebase.appdistribution.impl; +import androidx.annotation.VisibleForTesting; import com.google.android.gms.tasks.Task; import com.google.android.gms.tasks.TaskCompletionSource; import com.google.firebase.appdistribution.UpdateTask; @@ -8,16 +9,16 @@ class SequentialReference { - interface SequentialReferenceConsumer { - void consume(T value); + interface SequentialReferenceProducer { + T produce(); } - interface SequentialReferenceTransformer { - T transform(T value); + interface SequentialReferenceConsumer { + void consume(T value); } - interface SequentialReferenceTaskTransformer { - Task transform(T value); + interface SequentialReferenceTransformer { + U transform(T value); } interface SequentialReferenceUpdateTaskTransformer { @@ -37,51 +38,33 @@ interface SequentialReferenceUpdateTaskTransformer { value = initialValue; } - T get() { - return value; - } - - void set(T newValue) { - sequentialExecutor.execute( - () -> { - value = newValue; - }); - } - - void consume(SequentialReferenceConsumer listener) { + Task set(SequentialReferenceProducer producer) { + TaskCompletionSource taskCompletionSource = new TaskCompletionSource<>(); sequentialExecutor.execute( () -> { - listener.consume(value); + value = producer.produce(); + taskCompletionSource.setResult(value); }); + return taskCompletionSource.getTask(); } - void transform(SequentialReferenceTransformer transformer) { + Task setAndTransform(SequentialReferenceProducer producer, SequentialReferenceTransformer transformer) { + TaskCompletionSource taskCompletionSource = new TaskCompletionSource<>(); sequentialExecutor.execute( () -> { - value = transformer.transform(value); + value = producer.produce(); + taskCompletionSource.setResult(transformer.transform(value)); }); + return taskCompletionSource.getTask(); } - Task applyTask(SequentialReferenceTaskTransformer transformer) { - TaskCompletionSource taskCompletionSource = new TaskCompletionSource<>(); - sequentialExecutor.execute( - () -> - transformer - .transform(value) - .addOnSuccessListener(sequentialExecutor, taskCompletionSource::setResult) - .addOnFailureListener(sequentialExecutor, taskCompletionSource::setException)); - return taskCompletionSource.getTask(); + void get(SequentialReferenceConsumer consumer) { + sequentialExecutor.execute(() -> consumer.consume(value)); } - UpdateTask applyUpdateTask(SequentialReferenceUpdateTaskTransformer transformer) { + UpdateTask getAndTransform(SequentialReferenceUpdateTaskTransformer transformer) { UpdateTaskImpl updateTask = new UpdateTaskImpl(); - sequentialExecutor.execute( - () -> - transformer - .transform(value) - .addOnProgressListener(sequentialExecutor, updateTask::updateProgress) - .addOnSuccessListener(sequentialExecutor, result -> updateTask.setResult()) - .addOnFailureListener(sequentialExecutor, updateTask::setException)); + sequentialExecutor.execute(() -> updateTask.shadow(transformer.transform(value))); return updateTask; } } diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/UpdateTaskImpl.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/UpdateTaskImpl.java index b0f523951d4..d9dfb9209fd 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/UpdateTaskImpl.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/UpdateTaskImpl.java @@ -31,6 +31,7 @@ import com.google.firebase.appdistribution.OnProgressListener; import com.google.firebase.appdistribution.UpdateProgress; import com.google.firebase.appdistribution.UpdateTask; +import com.google.firebase.concurrent.FirebaseExecutors; import java.util.concurrent.Executor; /** Implementation of UpdateTask, the return type of updateApp. */ @@ -64,6 +65,13 @@ void updateProgress(@NonNull UpdateProgress updateProgress) { } } + void shadow(UpdateTask updateTask) { + updateTask + .addOnProgressListener(FirebaseExecutors.directExecutor(), this::updateProgress) + .addOnSuccessListener(FirebaseExecutors.directExecutor(), unused -> this.setResult()) + .addOnFailureListener(FirebaseExecutors.directExecutor(), this::setException); + } + private Task getTask() { synchronized (this.taskCompletionLock) { return this.taskCompletionSource.getTask(); From 8c8ac7b0ece44fffeee2205ea0b81e116cf27310 Mon Sep 17 00:00:00 2001 From: Lee Kellogg Date: Wed, 7 Dec 2022 15:20:22 -0500 Subject: [PATCH 05/12] Java formatting --- .../impl/AppDistributionReleaseInternal.java | 2 +- .../impl/FirebaseAppDistributionImpl.java | 21 ++++--- .../impl/SequentialReference.java | 4 +- ...irebaseAppDistributionServiceImplTest.java | 58 ++++++++++++------- 4 files changed, 50 insertions(+), 35 deletions(-) diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/AppDistributionReleaseInternal.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/AppDistributionReleaseInternal.java index ebababf1148..f16f1ea5f89 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/AppDistributionReleaseInternal.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/AppDistributionReleaseInternal.java @@ -22,7 +22,7 @@ /** * This class represents the AppDistributionRelease object returned by the App Distribution backend. * - * TODO(lkellogg): Combine this with AppDistributionReleaseImpl + *

TODO(lkellogg): Combine this with AppDistributionReleaseImpl */ @AutoValue abstract class AppDistributionReleaseInternal { diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java index ce57a395d3b..84205216ee1 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java @@ -217,10 +217,11 @@ public Task signInTester() { @Override public void signOutTester() { - cachedNewRelease.set(() -> { - signInStorage.setSignInStatus(false); - return null; - }); + cachedNewRelease.set( + () -> { + signInStorage.setSignInStatus(false); + return null; + }); } @Override @@ -230,8 +231,7 @@ public void signOutTester() { public synchronized Task checkForNewRelease() { if (!isTesterSignedIn()) { return Tasks.forException( - new FirebaseAppDistributionException( - "Tester is not signed in", AUTHENTICATION_FAILURE)); + new FirebaseAppDistributionException("Tester is not signed in", AUTHENTICATION_FAILURE)); } return checkForNewReleaseTaskCache.getOrCreateTask( @@ -242,8 +242,7 @@ public synchronized Task checkForNewRelease() { lightweightExecutor, release -> cachedNewRelease.setAndTransform( - () -> release, - ReleaseUtils::convertToAppDistributionRelease)) + () -> release, ReleaseUtils::convertToAppDistributionRelease)) .addOnFailureListener( lightweightExecutor, e -> { @@ -251,7 +250,8 @@ public synchronized Task checkForNewRelease() { && ((FirebaseAppDistributionException) e).getErrorCode() == AUTHENTICATION_FAILURE) { // If CheckForNewRelease returns authentication error, the FID is no longer - // valid or does not have access to the latest release. So sign out the tester + // valid or does not have access to the latest release. So sign out the + // tester // to force FID re-registration signOutTester(); } @@ -272,8 +272,7 @@ private UpdateTask updateApp(boolean showDownloadInNotificationManager) { if (!isTesterSignedIn()) { UpdateTaskImpl updateTask = new UpdateTaskImpl(); updateTask.setException( - new FirebaseAppDistributionException( - "Tester is not signed in", AUTHENTICATION_FAILURE)); + new FirebaseAppDistributionException("Tester is not signed in", AUTHENTICATION_FAILURE)); return updateTask; } return cachedNewRelease.getAndTransform( diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java index d64885cc8d2..8b0ff937701 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java @@ -1,6 +1,5 @@ package com.google.firebase.appdistribution.impl; -import androidx.annotation.VisibleForTesting; import com.google.android.gms.tasks.Task; import com.google.android.gms.tasks.TaskCompletionSource; import com.google.firebase.appdistribution.UpdateTask; @@ -48,7 +47,8 @@ Task set(SequentialReferenceProducer producer) { return taskCompletionSource.getTask(); } - Task setAndTransform(SequentialReferenceProducer producer, SequentialReferenceTransformer transformer) { + Task setAndTransform( + SequentialReferenceProducer producer, SequentialReferenceTransformer transformer) { TaskCompletionSource taskCompletionSource = new TaskCompletionSource<>(); sequentialExecutor.execute( () -> { diff --git a/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java b/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java index 84fa1f67127..7084a13f26e 100644 --- a/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java +++ b/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java @@ -14,7 +14,6 @@ package com.google.firebase.appdistribution.impl; -import static android.os.Looper.getMainLooper; import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.AUTHENTICATION_CANCELED; import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.AUTHENTICATION_FAILURE; import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.HOST_ACTIVITY_INTERRUPTED; @@ -65,7 +64,6 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.ExecutionException; -import java.util.concurrent.Executor; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; @@ -187,7 +185,8 @@ public void setup() throws FirebaseAppDistributionException { } @Test - public void checkForNewRelease_whenCheckForNewReleaseFails_throwsError() throws InterruptedException { + public void checkForNewRelease_whenCheckForNewReleaseFails_throwsError() + throws InterruptedException { when(mockNewReleaseFetcher.checkForNewRelease()) .thenReturn( Tasks.forException( @@ -211,7 +210,8 @@ public void checkForNewRelease_testerIsNotSignedIn_taskFails() throws Interrupte } @Test - public void checkForNewRelease_whenCheckForNewReleaseSucceeds_returnsRelease() throws InterruptedException { + public void checkForNewRelease_whenCheckForNewReleaseSucceeds_returnsRelease() + throws InterruptedException { when(mockNewReleaseFetcher.checkForNewRelease()) .thenReturn( Tasks.forResult( @@ -251,7 +251,8 @@ public void updateApp_whenNotSignedIn_throwsError() throws InterruptedException } @Test - public void updateIfNewReleaseAvailable_whenNewAabReleaseAvailable_showsUpdateDialog() throws InterruptedException { + public void updateIfNewReleaseAvailable_whenNewAabReleaseAvailable_showsUpdateDialog() + throws InterruptedException { when(mockNewReleaseFetcher.checkForNewRelease()) .thenReturn(Tasks.forResult((TEST_RELEASE_NEWER_AAB_INTERNAL.build()))); @@ -282,7 +283,8 @@ public void updateIfNewReleaseAvailable_fromABackgroundThread_showsUpdateDialog( } @Test - public void updateIfNewReleaseAvailable_whenReleaseNotesEmpty_doesNotShowReleaseNotes() throws InterruptedException { + public void updateIfNewReleaseAvailable_whenReleaseNotesEmpty_doesNotShowReleaseNotes() + throws InterruptedException { when(mockNewReleaseFetcher.checkForNewRelease()) .thenReturn(Tasks.forResult((TEST_RELEASE_NEWER_AAB_INTERNAL.setReleaseNotes("").build()))); @@ -298,7 +300,8 @@ public void updateIfNewReleaseAvailable_whenReleaseNotesEmpty_doesNotShowRelease } @Test - public void updateIfNewReleaseAvailable_whenNoReleaseAvailable_updateDialogNotShown() throws InterruptedException { + public void updateIfNewReleaseAvailable_whenNoReleaseAvailable_updateDialogNotShown() + throws InterruptedException { when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(null)); UpdateTask task = firebaseAppDistribution.updateIfNewReleaseAvailable(); @@ -313,7 +316,8 @@ public void updateIfNewReleaseAvailable_whenNoReleaseAvailable_updateDialogNotSh } @Test - public void updateIfNewReleaseAvailable_whenActivityBackgrounded_updateDialogNotShown() throws InterruptedException { + public void updateIfNewReleaseAvailable_whenActivityBackgrounded_updateDialogNotShown() + throws InterruptedException { when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(null)); UpdateTask task = firebaseAppDistribution.updateIfNewReleaseAvailable(); @@ -328,7 +332,8 @@ public void updateIfNewReleaseAvailable_whenActivityBackgrounded_updateDialogNot } @Test - public void updateIfNewReleaseAvailable_whenSignInCancelled_checkForUpdateNotCalled() throws InterruptedException { + public void updateIfNewReleaseAvailable_whenSignInCancelled_checkForUpdateNotCalled() + throws InterruptedException { when(mockSignInStorage.getSignInStatus()).thenReturn(false); when(mockTesterSignInManager.signInTester()) .thenReturn( @@ -348,7 +353,8 @@ public void updateIfNewReleaseAvailable_whenSignInCancelled_checkForUpdateNotCal } @Test - public void updateIfNewReleaseAvailable_whenSignInFailed_checkForUpdateNotCalled() throws InterruptedException { + public void updateIfNewReleaseAvailable_whenSignInFailed_checkForUpdateNotCalled() + throws InterruptedException { when(mockSignInStorage.getSignInStatus()).thenReturn(false); when(mockTesterSignInManager.signInTester()) .thenReturn( @@ -366,7 +372,8 @@ public void updateIfNewReleaseAvailable_whenSignInFailed_checkForUpdateNotCalled } @Test - public void updateIfNewReleaseAvailable_whenDialogDismissed_taskFails() throws InterruptedException { + public void updateIfNewReleaseAvailable_whenDialogDismissed_taskFails() + throws InterruptedException { when(mockNewReleaseFetcher.checkForNewRelease()) .thenReturn(Tasks.forResult(TEST_RELEASE_NEWER_AAB_INTERNAL.build())); @@ -383,7 +390,8 @@ public void updateIfNewReleaseAvailable_whenDialogDismissed_taskFails() throws I } @Test - public void updateIfNewReleaseAvailable_whenDialogCanceled_taskFails() throws InterruptedException { + public void updateIfNewReleaseAvailable_whenDialogCanceled_taskFails() + throws InterruptedException { when(mockNewReleaseFetcher.checkForNewRelease()) .thenReturn(Tasks.forResult(TEST_RELEASE_NEWER_AAB_INTERNAL.build())); @@ -400,7 +408,8 @@ public void updateIfNewReleaseAvailable_whenDialogCanceled_taskFails() throws In } @Test - public void updateIfNewReleaseAvailable_whenCheckForUpdateFails_updateAppNotCalled() throws InterruptedException { + public void updateIfNewReleaseAvailable_whenCheckForUpdateFails_updateAppNotCalled() + throws InterruptedException { when(mockNewReleaseFetcher.checkForNewRelease()) .thenReturn( Tasks.forException( @@ -421,7 +430,8 @@ public void updateIfNewReleaseAvailable_whenCheckForUpdateFails_updateAppNotCall } @Test - public void updateIfNewReleaseAvailable_whenTesterIsSignedIn_doesNotOpenDialog() throws InterruptedException { + public void updateIfNewReleaseAvailable_whenTesterIsSignedIn_doesNotOpenDialog() + throws InterruptedException { when(mockSignInStorage.getSignInStatus()).thenReturn(true); firebaseAppDistribution.updateIfNewReleaseAvailable(); @@ -447,7 +457,8 @@ public void signInTester_whenDialogDismissed_taskFails() throws InterruptedExcep } @Test - public void updateIfNewReleaseAvailable_whenSignInDialogCanceled_taskFails() throws InterruptedException { + public void updateIfNewReleaseAvailable_whenSignInDialogCanceled_taskFails() + throws InterruptedException { when(mockSignInStorage.getSignInStatus()).thenReturn(false); Task signInTask = firebaseAppDistribution.updateIfNewReleaseAvailable(); @@ -477,7 +488,8 @@ public void signOutTester_setsSignInStatusFalse() { } @Test - public void updateIfNewReleaseAvailable_receiveProgressUpdateFromUpdateApp() throws InterruptedException { + public void updateIfNewReleaseAvailable_receiveProgressUpdateFromUpdateApp() + throws InterruptedException { AppDistributionReleaseInternal newRelease = TEST_RELEASE_NEWER_AAB_INTERNAL.build(); when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(newRelease)); UpdateTaskImpl updateTaskToReturn = new UpdateTaskImpl(); @@ -499,7 +511,7 @@ public void updateIfNewReleaseAvailable_receiveProgressUpdateFromUpdateApp() thr AlertDialog updateDialog = (AlertDialog) ShadowAlertDialog.getLatestDialog(); updateDialog.getButton(Dialog.BUTTON_POSITIVE).performClick(); TestUtils.awaitAsyncOperations(testExecutor); -// TestUtils.awaitAsyncOperations(testExecutor); + // TestUtils.awaitAsyncOperations(testExecutor); // Update flow assertEquals(1, progressEvents.size()); @@ -521,7 +533,8 @@ public void updateIfNewReleaseAvailable_fromABackgroundThread_showsSignInDialog( } @Test - public void updateIfNewReleaseAvailable_whenScreenRotates_signInConfirmationDialogReappears() throws InterruptedException { + public void updateIfNewReleaseAvailable_whenScreenRotates_signInConfirmationDialogReappears() + throws InterruptedException { when(mockSignInStorage.getSignInStatus()).thenReturn(false); when(activity.isChangingConfigurations()).thenReturn(true); @@ -537,7 +550,8 @@ public void updateIfNewReleaseAvailable_whenScreenRotates_signInConfirmationDial } @Test - public void updateIfNewReleaseAvailable_whenScreenRotates_updateDialogReappears() throws InterruptedException { + public void updateIfNewReleaseAvailable_whenScreenRotates_updateDialogReappears() + throws InterruptedException { AppDistributionReleaseInternal newRelease = TEST_RELEASE_NEWER_AAB_INTERNAL.build(); when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(newRelease)); when(activity.isChangingConfigurations()).thenReturn(true); @@ -555,7 +569,8 @@ public void updateIfNewReleaseAvailable_whenScreenRotates_updateDialogReappears( @Test public void - updateIfNewReleaseAvailable_whenSignInDialogShowingAndNewActivityStarts_signInTaskCancelled() throws InterruptedException { + updateIfNewReleaseAvailable_whenSignInDialogShowingAndNewActivityStarts_signInTaskCancelled() + throws InterruptedException { TestActivity testActivity2 = new TestActivity(); when(mockSignInStorage.getSignInStatus()).thenReturn(false); @@ -572,7 +587,8 @@ public void updateIfNewReleaseAvailable_whenScreenRotates_updateDialogReappears( @Test public void - updateIfNewReleaseAvailable_whenUpdateDialogShowingAndNewActivityStarts_updateTaskCancelled() throws InterruptedException { + updateIfNewReleaseAvailable_whenUpdateDialogShowingAndNewActivityStarts_updateTaskCancelled() + throws InterruptedException { TestActivity testActivity2 = new TestActivity(); AppDistributionReleaseInternal newRelease = TEST_RELEASE_NEWER_AAB_INTERNAL.build(); when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(newRelease)); From 1355cb10dd768c7f775c9946484e169b8823af73 Mon Sep 17 00:00:00 2001 From: Lee Kellogg Date: Wed, 7 Dec 2022 15:53:38 -0500 Subject: [PATCH 06/12] Add tests --- .../impl/FirebaseAppDistributionImpl.java | 7 ++--- .../impl/SequentialReference.java | 19 +++++++------- .../appdistribution/impl/UpdateTaskImpl.java | 9 +++---- ...irebaseAppDistributionServiceImplTest.java | 26 ++++++++++++++----- 4 files changed, 36 insertions(+), 25 deletions(-) diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java index 84205216ee1..848e914ba11 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java @@ -217,11 +217,8 @@ public Task signInTester() { @Override public void signOutTester() { - cachedNewRelease.set( - () -> { - signInStorage.setSignInStatus(false); - return null; - }); + cachedNewRelease.set(null); + signInStorage.setSignInStatus(false); } @Override diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java index 8b0ff937701..690a56795f2 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java @@ -1,5 +1,6 @@ package com.google.firebase.appdistribution.impl; +import androidx.annotation.VisibleForTesting; import com.google.android.gms.tasks.Task; import com.google.android.gms.tasks.TaskCompletionSource; import com.google.firebase.appdistribution.UpdateTask; @@ -37,14 +38,8 @@ interface SequentialReferenceUpdateTaskTransformer { value = initialValue; } - Task set(SequentialReferenceProducer producer) { - TaskCompletionSource taskCompletionSource = new TaskCompletionSource<>(); - sequentialExecutor.execute( - () -> { - value = producer.produce(); - taskCompletionSource.setResult(value); - }); - return taskCompletionSource.getTask(); + void set(T newValue) { + sequentialExecutor.execute(() -> value = newValue); } Task setAndTransform( @@ -64,7 +59,13 @@ void get(SequentialReferenceConsumer consumer) { UpdateTask getAndTransform(SequentialReferenceUpdateTaskTransformer transformer) { UpdateTaskImpl updateTask = new UpdateTaskImpl(); - sequentialExecutor.execute(() -> updateTask.shadow(transformer.transform(value))); + sequentialExecutor.execute( + () -> updateTask.shadow(sequentialExecutor, transformer.transform(value))); return updateTask; } + + @VisibleForTesting + T getSnapshot() { + return value; + } } diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/UpdateTaskImpl.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/UpdateTaskImpl.java index d9dfb9209fd..4840d3ae038 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/UpdateTaskImpl.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/UpdateTaskImpl.java @@ -31,7 +31,6 @@ import com.google.firebase.appdistribution.OnProgressListener; import com.google.firebase.appdistribution.UpdateProgress; import com.google.firebase.appdistribution.UpdateTask; -import com.google.firebase.concurrent.FirebaseExecutors; import java.util.concurrent.Executor; /** Implementation of UpdateTask, the return type of updateApp. */ @@ -65,11 +64,11 @@ void updateProgress(@NonNull UpdateProgress updateProgress) { } } - void shadow(UpdateTask updateTask) { + void shadow(Executor executor, UpdateTask updateTask) { updateTask - .addOnProgressListener(FirebaseExecutors.directExecutor(), this::updateProgress) - .addOnSuccessListener(FirebaseExecutors.directExecutor(), unused -> this.setResult()) - .addOnFailureListener(FirebaseExecutors.directExecutor(), this::setException); + .addOnProgressListener(executor, this::updateProgress) + .addOnSuccessListener(executor, unused -> this.setResult()) + .addOnFailureListener(executor, this::setException); } private Task getTask() { diff --git a/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java b/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java index 7084a13f26e..c89201982b6 100644 --- a/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java +++ b/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java @@ -224,7 +224,7 @@ public void checkForNewRelease_whenCheckForNewReleaseSucceeds_returnsRelease() assertEquals(TEST_RELEASE_NEWER_AAB, task.getResult()); assertEquals( TEST_RELEASE_NEWER_AAB_INTERNAL.build(), - firebaseAppDistribution.getCachedNewRelease().get()); + firebaseAppDistribution.getCachedNewRelease().getSnapshot()); } @Test @@ -401,6 +401,7 @@ public void updateIfNewReleaseAvailable_whenDialogCanceled_taskFails() AlertDialog updateDialog = assertAlertDialogShown(); updateDialog.onBackPressed(); // cancels the dialog TestUtils.awaitAsyncOperations(testExecutor); + TestUtils.awaitAsyncOperations(testExecutor); assertFalse(updateDialog.isShowing()); @@ -465,7 +466,9 @@ public void updateIfNewReleaseAvailable_whenSignInDialogCanceled_taskFails() AlertDialog dialog = assertAlertDialogShown(); dialog.onBackPressed(); // cancel dialog TestUtils.awaitAsyncOperations(testExecutor); + TestUtils.awaitAsyncOperations(testExecutor); + assertTrue(signInTask.isComplete()); assertFalse(signInTask.isSuccessful()); Exception e = signInTask.getException(); assertTrue(e instanceof FirebaseAppDistributionException); @@ -511,7 +514,6 @@ public void updateIfNewReleaseAvailable_receiveProgressUpdateFromUpdateApp() AlertDialog updateDialog = (AlertDialog) ShadowAlertDialog.getLatestDialog(); updateDialog.getButton(Dialog.BUTTON_POSITIVE).performClick(); TestUtils.awaitAsyncOperations(testExecutor); - // TestUtils.awaitAsyncOperations(testExecutor); // Update flow assertEquals(1, progressEvents.size()); @@ -617,7 +619,7 @@ public void updateAppTask_whenNoReleaseAvailable_throwsError() throws Interrupte } @Test - public void updateApp_withAabReleaseAvailable_returnsSameAabTask() { + public void updateApp_withAabReleaseAvailable_returnsSameAabTask() throws InterruptedException { AppDistributionReleaseInternal release = TEST_RELEASE_NEWER_AAB_INTERNAL.build(); firebaseAppDistribution.getCachedNewRelease().set(release); UpdateTaskImpl updateTaskToReturn = new UpdateTaskImpl(); @@ -625,12 +627,18 @@ public void updateApp_withAabReleaseAvailable_returnsSameAabTask() { when(mockSignInStorage.getSignInStatus()).thenReturn(true); UpdateTask updateTask = firebaseAppDistribution.updateApp(); + assertFalse(updateTask.isComplete()); + + // Complete original task + updateTaskToReturn.setResult(); + TestUtils.awaitAsyncOperations(testExecutor); - assertEquals(updateTask, updateTaskToReturn); + // Returned task is complete + assertTrue(updateTask.isSuccessful()); } @Test - public void updateApp_withApkReleaseAvailable_returnsSameApkTask() { + public void updateApp_withApkReleaseAvailable_returnsSameApkTask() throws InterruptedException { when(mockSignInStorage.getSignInStatus()).thenReturn(true); AppDistributionReleaseInternal release = TEST_RELEASE_NEWER_APK_INTERNAL.build(); firebaseAppDistribution.getCachedNewRelease().set(release); @@ -638,7 +646,13 @@ public void updateApp_withApkReleaseAvailable_returnsSameApkTask() { doReturn(updateTaskToReturn).when(mockApkUpdater).updateApk(release, false); UpdateTask updateTask = firebaseAppDistribution.updateApp(); + assertFalse(updateTask.isComplete()); + + // Complete original task + updateTaskToReturn.setResult(); + TestUtils.awaitAsyncOperations(testExecutor); - assertEquals(updateTask, updateTaskToReturn); + // Returned task is complete + assertTrue(updateTask.isSuccessful()); } } From 928ff1a3706cd8b7be2d663128fe0fe129d96891 Mon Sep 17 00:00:00 2001 From: Lee Kellogg Date: Thu, 8 Dec 2022 10:48:47 -0500 Subject: [PATCH 07/12] Add javadocs --- .../impl/FirebaseAppDistributionImpl.java | 5 +-- .../impl/SequentialReference.java | 37 +++++++++++++++---- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java index 848e914ba11..1e6d79a7faf 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java @@ -239,7 +239,7 @@ public synchronized Task checkForNewRelease() { lightweightExecutor, release -> cachedNewRelease.setAndTransform( - () -> release, ReleaseUtils::convertToAppDistributionRelease)) + release, ReleaseUtils::convertToAppDistributionRelease)) .addOnFailureListener( lightweightExecutor, e -> { @@ -248,8 +248,7 @@ public synchronized Task checkForNewRelease() { == AUTHENTICATION_FAILURE) { // If CheckForNewRelease returns authentication error, the FID is no longer // valid or does not have access to the latest release. So sign out the - // tester - // to force FID re-registration + // tester to force FID re-registration signOutTester(); } })); diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java index 690a56795f2..edb59cd0e82 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java @@ -1,3 +1,17 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package com.google.firebase.appdistribution.impl; import androidx.annotation.VisibleForTesting; @@ -7,12 +21,12 @@ import com.google.firebase.concurrent.FirebaseExecutors; import java.util.concurrent.Executor; +/** + * A reference to an object that uses a sequential executor to ensure non-concurrent reads and + * writes while preventing lock contention. + */ class SequentialReference { - interface SequentialReferenceProducer { - T produce(); - } - interface SequentialReferenceConsumer { void consume(T value); } @@ -38,25 +52,34 @@ interface SequentialReferenceUpdateTaskTransformer { value = initialValue; } + /** Enqueues a value to be set. */ void set(T newValue) { sequentialExecutor.execute(() -> value = newValue); } - Task setAndTransform( - SequentialReferenceProducer producer, SequentialReferenceTransformer transformer) { + /** + * Enqueues a new value to be set, and returns a {@link Task} that resolves with the result of + * applying the given {@code transformer} to the value. + */ + Task setAndTransform(T newValue, SequentialReferenceTransformer transformer) { TaskCompletionSource taskCompletionSource = new TaskCompletionSource<>(); sequentialExecutor.execute( () -> { - value = producer.produce(); + value = newValue; taskCompletionSource.setResult(transformer.transform(value)); }); return taskCompletionSource.getTask(); } + /** Gets the value, passing it to the given {@code consumer}. */ void get(SequentialReferenceConsumer consumer) { sequentialExecutor.execute(() -> consumer.consume(value)); } + /** + * Gets the value, returning an {@link UpdateTask} produced by applying the {@code transformer} + * function to the value. + */ UpdateTask getAndTransform(SequentialReferenceUpdateTaskTransformer transformer) { UpdateTaskImpl updateTask = new UpdateTaskImpl(); sequentialExecutor.execute( From f618340a9013d1ebe13d433eccbbd4c9e032867f Mon Sep 17 00:00:00 2001 From: Lee Kellogg Date: Thu, 8 Dec 2022 18:48:40 -0500 Subject: [PATCH 08/12] Simplify SequentialReference and update tests --- .../impl/AppDistributionReleaseInternal.java | 3 + .../impl/FirebaseAppDistributionImpl.java | 29 +-- .../impl/SequentialReference.java | 66 +++---- .../appdistribution/impl/TaskUtils.java | 32 ++++ .../appdistribution/impl/UpdateTaskImpl.java | 13 +- ...irebaseAppDistributionServiceImplTest.java | 181 ++++++++---------- .../appdistribution/impl/TestUtils.java | 6 +- 7 files changed, 172 insertions(+), 158 deletions(-) diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/AppDistributionReleaseInternal.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/AppDistributionReleaseInternal.java index f16f1ea5f89..20bb24e4bff 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/AppDistributionReleaseInternal.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/AppDistributionReleaseInternal.java @@ -67,6 +67,9 @@ static Builder builder() { @Nullable abstract String getDownloadUrl(); + @NonNull + abstract Builder toBuilder(); + /** Builder for {@link AppDistributionReleaseInternal}. */ @AutoValue.Builder abstract static class Builder { diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java index 1e6d79a7faf..2654b55c244 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java @@ -40,6 +40,7 @@ import com.google.firebase.appdistribution.UpdateProgress; import com.google.firebase.appdistribution.UpdateStatus; import com.google.firebase.appdistribution.UpdateTask; +import com.google.firebase.concurrent.FirebaseExecutors; import java.util.concurrent.Executor; /** @@ -88,7 +89,7 @@ class FirebaseAppDistributionImpl implements FirebaseAppDistribution { this.aabUpdater = aabUpdater; this.signInStorage = signInStorage; this.lifecycleNotifier = lifecycleNotifier; - this.cachedNewRelease = new SequentialReference<>(lightweightExecutor); + this.cachedNewRelease = SequentialReference.withBaseExecutor(lightweightExecutor); this.lightweightExecutor = lightweightExecutor; lifecycleNotifier.addOnActivityDestroyedListener(this::onActivityDestroyed); lifecycleNotifier.addOnActivityPausedListener(this::onActivityPaused); @@ -217,8 +218,10 @@ public Task signInTester() { @Override public void signOutTester() { - cachedNewRelease.set(null); - signInStorage.setSignInStatus(false); + cachedNewRelease + .set(null) + .addOnSuccessListener( + FirebaseExecutors.directExecutor(), unused -> signInStorage.setSignInStatus(false)); } @Override @@ -235,11 +238,11 @@ public synchronized Task checkForNewRelease() { () -> newReleaseFetcher .checkForNewRelease() + .onSuccessTask(lightweightExecutor, release -> cachedNewRelease.set(release)) .onSuccessTask( - lightweightExecutor, + FirebaseExecutors.directExecutor(), release -> - cachedNewRelease.setAndTransform( - release, ReleaseUtils::convertToAppDistributionRelease)) + Tasks.forResult(ReleaseUtils.convertToAppDistributionRelease(release))) .addOnFailureListener( lightweightExecutor, e -> { @@ -271,7 +274,9 @@ private UpdateTask updateApp(boolean showDownloadInNotificationManager) { new FirebaseAppDistributionException("Tester is not signed in", AUTHENTICATION_FAILURE)); return updateTask; } - return cachedNewRelease.getAndTransform( + return TaskUtils.onSuccessUpdateTask( + cachedNewRelease.get(), + lightweightExecutor, release -> { if (release == null) { LogWrapper.getInstance().v("New release not found."); @@ -313,10 +318,12 @@ void onActivityResumed(Activity activity) { new FirebaseAppDistributionException( ErrorMessages.HOST_ACTIVITY_INTERRUPTED, HOST_ACTIVITY_INTERRUPTED)); } else { - cachedNewRelease.get( - release -> - showUpdateConfirmationDialog( - activity, ReleaseUtils.convertToAppDistributionRelease(release))); + cachedNewRelease + .get() + .addOnSuccessListener( + release -> + showUpdateConfirmationDialog( + activity, ReleaseUtils.convertToAppDistributionRelease(release))); } } } diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java index edb59cd0e82..c6ebf93e2f8 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java @@ -15,9 +15,9 @@ package com.google.firebase.appdistribution.impl; import androidx.annotation.VisibleForTesting; +import com.google.android.gms.tasks.OnSuccessListener; import com.google.android.gms.tasks.Task; import com.google.android.gms.tasks.TaskCompletionSource; -import com.google.firebase.appdistribution.UpdateTask; import com.google.firebase.concurrent.FirebaseExecutors; import java.util.concurrent.Executor; @@ -27,64 +27,52 @@ */ class SequentialReference { - interface SequentialReferenceConsumer { - void consume(T value); - } - - interface SequentialReferenceTransformer { - U transform(T value); - } - - interface SequentialReferenceUpdateTaskTransformer { - UpdateTask transform(T value); - } - private final Executor sequentialExecutor; - private T value; - SequentialReference(Executor baseExecutor) { - this(baseExecutor, null); + /** Get a {@link SequentialReference} that controls access uses the given sequential executor. */ + static SequentialReference withSequentialExecutor(Executor sequentialExecutor) { + return new SequentialReference(sequentialExecutor); } - SequentialReference(Executor baseExecutor, T initialValue) { - sequentialExecutor = FirebaseExecutors.newSequentialExecutor(baseExecutor); - value = initialValue; + /** + * Get a {@link SequentialReference} that controls access using its own sequential executor backed + * by the given base executor. + */ + static SequentialReference withBaseExecutor(Executor baseExecutor) { + return new SequentialReference(FirebaseExecutors.newSequentialExecutor(baseExecutor)); } - /** Enqueues a value to be set. */ - void set(T newValue) { - sequentialExecutor.execute(() -> value = newValue); + private SequentialReference(Executor sequentialExecutor) { + this.sequentialExecutor = sequentialExecutor; } /** - * Enqueues a new value to be set, and returns a {@link Task} that resolves with the result of - * applying the given {@code transformer} to the value. + * Sets the value, returning a {@link Task} will complete once it is set. + * + *

For convenience when chaining tasks together, the result of the returned task is the value. */ - Task setAndTransform(T newValue, SequentialReferenceTransformer transformer) { - TaskCompletionSource taskCompletionSource = new TaskCompletionSource<>(); + Task set(T newValue) { + TaskCompletionSource taskCompletionSource = new TaskCompletionSource<>(); sequentialExecutor.execute( () -> { value = newValue; - taskCompletionSource.setResult(transformer.transform(value)); + taskCompletionSource.setResult(value); }); return taskCompletionSource.getTask(); } - /** Gets the value, passing it to the given {@code consumer}. */ - void get(SequentialReferenceConsumer consumer) { - sequentialExecutor.execute(() -> consumer.consume(value)); - } - /** - * Gets the value, returning an {@link UpdateTask} produced by applying the {@code transformer} - * function to the value. + * Gets a {@link Task} that will complete with the value. + * + *

Unless a direct executer is passed to {@link Task#addOnSuccessListener(Executor, + * OnSuccessListener)} or similar methods, be aware that the value may have changed by the time it + * is used. */ - UpdateTask getAndTransform(SequentialReferenceUpdateTaskTransformer transformer) { - UpdateTaskImpl updateTask = new UpdateTaskImpl(); - sequentialExecutor.execute( - () -> updateTask.shadow(sequentialExecutor, transformer.transform(value))); - return updateTask; + Task get() { + TaskCompletionSource taskCompletionSource = new TaskCompletionSource<>(); + sequentialExecutor.execute(() -> taskCompletionSource.setResult(value)); + return taskCompletionSource.getTask(); } @VisibleForTesting diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/TaskUtils.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/TaskUtils.java index 28757527e5c..3b39be97dc6 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/TaskUtils.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/TaskUtils.java @@ -14,11 +14,13 @@ package com.google.firebase.appdistribution.impl; +import com.google.android.gms.tasks.SuccessContinuation; import com.google.android.gms.tasks.Task; import com.google.android.gms.tasks.TaskCompletionSource; import com.google.android.gms.tasks.Tasks; import com.google.firebase.appdistribution.FirebaseAppDistributionException; import com.google.firebase.appdistribution.FirebaseAppDistributionException.Status; +import com.google.firebase.appdistribution.UpdateTask; import java.util.concurrent.Executor; class TaskUtils { @@ -32,6 +34,14 @@ interface Operation { TResult run() throws FirebaseAppDistributionException; } + /** + * A function that is called to continue execution when a {@link Task} succeeds, and returns an + * {@link UpdateTask}. + */ + interface UpdateTaskContinuation { + UpdateTask then(TResult result) throws FirebaseAppDistributionException; + } + /** * Runs a long running operation inside a {@link Task}, wrapping any errors in {@link * FirebaseAppDistributionException}. @@ -111,5 +121,27 @@ static void safeSetTaskResult(UpdateTaskImpl task) { } } + /** + * Returns an {@link UpdateTask} that will be completed with the result of applying the specified + * {@link UpdateTaskContinuation} to the given {@link Task} when the task completes successfully. + * + *

This is equivalent to {@link Task#onSuccessTask(Executor, SuccessContinuation)} but for a + * continuation that returns an {@link UpdateTask}. + */ + static UpdateTask onSuccessUpdateTask( + Task task, Executor executor, UpdateTaskContinuation continuation) { + UpdateTaskImpl updateTask = new UpdateTaskImpl(); + task.addOnSuccessListener( + executor, + result -> { + try { + updateTask.shadow(continuation.then(result)); + } catch (Throwable t) { + updateTask.setException(FirebaseAppDistributionExceptions.wrap(t)); + } + }); + return updateTask; + } + private TaskUtils() {} } diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/UpdateTaskImpl.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/UpdateTaskImpl.java index 4840d3ae038..8b9f45003e2 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/UpdateTaskImpl.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/UpdateTaskImpl.java @@ -31,6 +31,7 @@ import com.google.firebase.appdistribution.OnProgressListener; import com.google.firebase.appdistribution.UpdateProgress; import com.google.firebase.appdistribution.UpdateTask; +import com.google.firebase.concurrent.FirebaseExecutors; import java.util.concurrent.Executor; /** Implementation of UpdateTask, the return type of updateApp. */ @@ -64,11 +65,15 @@ void updateProgress(@NonNull UpdateProgress updateProgress) { } } - void shadow(Executor executor, UpdateTask updateTask) { + /** + * Listen to all completion and progress events on the given {@link UpdateTask}, updating the + * progress or completing this task with the same changes. + */ + void shadow(UpdateTask updateTask) { updateTask - .addOnProgressListener(executor, this::updateProgress) - .addOnSuccessListener(executor, unused -> this.setResult()) - .addOnFailureListener(executor, this::setException); + .addOnProgressListener(FirebaseExecutors.directExecutor(), this::updateProgress) + .addOnSuccessListener(FirebaseExecutors.directExecutor(), unused -> this.setResult()) + .addOnFailureListener(FirebaseExecutors.directExecutor(), this::setException); } private Task getTask() { diff --git a/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java b/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java index c89201982b6..904b17c4ce7 100644 --- a/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java +++ b/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java @@ -14,6 +14,7 @@ package com.google.firebase.appdistribution.impl; +import static android.os.Looper.getMainLooper; import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.AUTHENTICATION_CANCELED; import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.AUTHENTICATION_FAILURE; import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.HOST_ACTIVITY_INTERRUPTED; @@ -25,10 +26,8 @@ import static com.google.firebase.appdistribution.impl.ErrorMessages.NETWORK_ERROR; import static com.google.firebase.appdistribution.impl.ErrorMessages.RELEASE_NOT_FOUND_ERROR; import static com.google.firebase.appdistribution.impl.ErrorMessages.UPDATE_CANCELED; -import static com.google.firebase.appdistribution.impl.TestUtils.assertTaskFailure; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.doReturn; @@ -60,6 +59,7 @@ import com.google.firebase.appdistribution.UpdateProgress; import com.google.firebase.appdistribution.UpdateStatus; import com.google.firebase.appdistribution.UpdateTask; +import com.google.firebase.concurrent.FirebaseExecutors; import com.google.firebase.installations.InstallationTokenResult; import java.util.ArrayList; import java.util.List; @@ -87,15 +87,15 @@ public class FirebaseAppDistributionServiceImplTest { private static final String IAS_ARTIFACT_ID_KEY = "com.android.vending.internal.apk.id"; private static final String TEST_URL = "https://test-url"; private static final long INSTALLED_VERSION_CODE = 2; - private static final String TEST_SCREENSHOT_FILE_NAME = "screenshot.png"; - private static final AppDistributionReleaseInternal.Builder TEST_RELEASE_NEWER_AAB_INTERNAL = + private static final AppDistributionReleaseInternal TEST_RELEASE_NEWER_AAB_INTERNAL = AppDistributionReleaseInternal.builder() .setBuildVersion("3") .setDisplayVersion("3.0") .setReleaseNotes("Newer version.") .setBinaryType(BinaryType.AAB) - .setDownloadUrl(TEST_URL); + .setDownloadUrl(TEST_URL) + .build(); private static final AppDistributionRelease TEST_RELEASE_NEWER_AAB = AppDistributionReleaseImpl.builder() @@ -105,13 +105,14 @@ public class FirebaseAppDistributionServiceImplTest { .setBinaryType(BinaryType.AAB) .build(); - private static final AppDistributionReleaseInternal.Builder TEST_RELEASE_NEWER_APK_INTERNAL = + private static final AppDistributionReleaseInternal TEST_RELEASE_NEWER_APK_INTERNAL = AppDistributionReleaseInternal.builder() .setBuildVersion("3") .setDisplayVersion("3.0") .setReleaseNotes("Newer version.") .setBinaryType(BinaryType.APK) - .setDownloadUrl(TEST_URL); + .setDownloadUrl(TEST_URL) + .build(); private final ExecutorService testExecutor = Executors.newSingleThreadExecutor(); @@ -126,7 +127,6 @@ public class FirebaseAppDistributionServiceImplTest { @Mock private AabUpdater mockAabUpdater; @Mock private SignInStorage mockSignInStorage; @Mock private FirebaseAppDistributionLifecycleNotifier mockLifecycleNotifier; - @Mock private ReleaseIdentifier mockReleaseIdentifier; static class TestActivity extends Activity {} @@ -185,8 +185,7 @@ public void setup() throws FirebaseAppDistributionException { } @Test - public void checkForNewRelease_whenCheckForNewReleaseFails_throwsError() - throws InterruptedException { + public void checkForNewRelease_whenCheckForNewReleaseFails_throwsError() { when(mockNewReleaseFetcher.checkForNewRelease()) .thenReturn( Tasks.forException( @@ -194,37 +193,30 @@ public void checkForNewRelease_whenCheckForNewReleaseFails_throwsError() ErrorMessages.JSON_PARSING_ERROR, Status.NETWORK_FAILURE))); Task task = firebaseAppDistribution.checkForNewRelease(); - TestUtils.awaitAsyncOperations(testExecutor); - assertTaskFailure(task, NETWORK_FAILURE, JSON_PARSING_ERROR); + TestUtils.awaitTaskFailure(task, NETWORK_FAILURE, JSON_PARSING_ERROR); } @Test - public void checkForNewRelease_testerIsNotSignedIn_taskFails() throws InterruptedException { + public void checkForNewRelease_testerIsNotSignedIn_taskFails() { when(firebaseAppDistribution.isTesterSignedIn()).thenReturn(false); Task task = firebaseAppDistribution.checkForNewRelease(); - TestUtils.awaitAsyncOperations(testExecutor); - assertTaskFailure(task, AUTHENTICATION_FAILURE, "Tester is not signed in"); + TestUtils.awaitTaskFailure(task, AUTHENTICATION_FAILURE, "Tester is not signed in"); } @Test public void checkForNewRelease_whenCheckForNewReleaseSucceeds_returnsRelease() - throws InterruptedException { - when(mockNewReleaseFetcher.checkForNewRelease()) - .thenReturn( - Tasks.forResult( - TEST_RELEASE_NEWER_AAB_INTERNAL.setReleaseNotes("Newer version.").build())); + throws InterruptedException, FirebaseAppDistributionException, ExecutionException { + AppDistributionReleaseInternal internalRelease = TEST_RELEASE_NEWER_AAB_INTERNAL; + when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(internalRelease)); Task task = firebaseAppDistribution.checkForNewRelease(); - TestUtils.awaitAsyncOperations(testExecutor); + TestUtils.awaitTask(task); - assertNotNull(task.getResult()); assertEquals(TEST_RELEASE_NEWER_AAB, task.getResult()); - assertEquals( - TEST_RELEASE_NEWER_AAB_INTERNAL.build(), - firebaseAppDistribution.getCachedNewRelease().getSnapshot()); + assertEquals(internalRelease, firebaseAppDistribution.getCachedNewRelease().getSnapshot()); } @Test @@ -234,27 +226,27 @@ public void checkForNewRelease_authenticationFailure_signOutTester() throws Inte Tasks.forException( new FirebaseAppDistributionException("Test", AUTHENTICATION_FAILURE))); - firebaseAppDistribution.checkForNewRelease(); - TestUtils.awaitAsyncOperations(testExecutor); + Task task = firebaseAppDistribution.checkForNewRelease(); + TestUtils.awaitTaskFailure(task, AUTHENTICATION_FAILURE, "Test"); + TestUtils.awaitTermination(testExecutor); verify(mockSignInStorage, times(1)).setSignInStatus(false); } @Test - public void updateApp_whenNotSignedIn_throwsError() throws InterruptedException { + public void updateApp_whenNotSignedIn_throwsError() { when(mockSignInStorage.getSignInStatus()).thenReturn(false); UpdateTask updateTask = firebaseAppDistribution.updateApp(); - TestUtils.awaitAsyncOperations(testExecutor); - assertTaskFailure(updateTask, AUTHENTICATION_FAILURE, "Tester is not signed in"); + TestUtils.awaitTaskFailure(updateTask, AUTHENTICATION_FAILURE, "Tester is not signed in"); } @Test public void updateIfNewReleaseAvailable_whenNewAabReleaseAvailable_showsUpdateDialog() throws InterruptedException { when(mockNewReleaseFetcher.checkForNewRelease()) - .thenReturn(Tasks.forResult((TEST_RELEASE_NEWER_AAB_INTERNAL.build()))); + .thenReturn(Tasks.forResult((TEST_RELEASE_NEWER_AAB_INTERNAL))); firebaseAppDistribution.updateIfNewReleaseAvailable(); TestUtils.awaitAsyncOperations(testExecutor); @@ -273,7 +265,7 @@ public void updateIfNewReleaseAvailable_whenNewAabReleaseAvailable_showsUpdateDi public void updateIfNewReleaseAvailable_fromABackgroundThread_showsUpdateDialog() throws InterruptedException { when(mockNewReleaseFetcher.checkForNewRelease()) - .thenReturn(Tasks.forResult((TEST_RELEASE_NEWER_AAB_INTERNAL.build()))); + .thenReturn(Tasks.forResult((TEST_RELEASE_NEWER_AAB_INTERNAL))); ExecutorService executorService = Executors.newSingleThreadExecutor(); executorService.submit(() -> firebaseAppDistribution.updateIfNewReleaseAvailable()); @@ -286,7 +278,9 @@ public void updateIfNewReleaseAvailable_fromABackgroundThread_showsUpdateDialog( public void updateIfNewReleaseAvailable_whenReleaseNotesEmpty_doesNotShowReleaseNotes() throws InterruptedException { when(mockNewReleaseFetcher.checkForNewRelease()) - .thenReturn(Tasks.forResult((TEST_RELEASE_NEWER_AAB_INTERNAL.setReleaseNotes("").build()))); + .thenReturn( + Tasks.forResult( + (TEST_RELEASE_NEWER_AAB_INTERNAL.toBuilder().setReleaseNotes("").build()))); firebaseAppDistribution.updateIfNewReleaseAvailable(); TestUtils.awaitAsyncOperations(testExecutor); @@ -307,8 +301,8 @@ public void updateIfNewReleaseAvailable_whenNoReleaseAvailable_updateDialogNotSh UpdateTask task = firebaseAppDistribution.updateIfNewReleaseAvailable(); List progressEvents = new ArrayList<>(); - task.addOnProgressListener(progressEvents::add); - TestUtils.awaitAsyncOperations(testExecutor); + task.addOnProgressListener(FirebaseExecutors.directExecutor(), progressEvents::add); + TestUtils.awaitTermination(testExecutor); assertEquals(1, progressEvents.size()); assertEquals(UpdateStatus.NEW_RELEASE_NOT_AVAILABLE, progressEvents.get(0).getUpdateStatus()); @@ -323,17 +317,16 @@ public void updateIfNewReleaseAvailable_whenActivityBackgrounded_updateDialogNot UpdateTask task = firebaseAppDistribution.updateIfNewReleaseAvailable(); List progressEvents = new ArrayList<>(); - task.addOnProgressListener(progressEvents::add); + task.addOnProgressListener(FirebaseExecutors.directExecutor(), progressEvents::add); + TestUtils.awaitTermination(testExecutor); - TestUtils.awaitAsyncOperations(testExecutor); assertEquals(1, progressEvents.size()); assertEquals(UpdateStatus.NEW_RELEASE_NOT_AVAILABLE, progressEvents.get(0).getUpdateStatus()); assertNull(ShadowAlertDialog.getLatestAlertDialog()); } @Test - public void updateIfNewReleaseAvailable_whenSignInCancelled_checkForUpdateNotCalled() - throws InterruptedException { + public void updateIfNewReleaseAvailable_whenSignInCancelled_checkForUpdateNotCalled() { when(mockSignInStorage.getSignInStatus()).thenReturn(false); when(mockTesterSignInManager.signInTester()) .thenReturn( @@ -346,15 +339,14 @@ public void updateIfNewReleaseAvailable_whenSignInCancelled_checkForUpdateNotCal AlertDialog signInDialog = assertAlertDialogShown(); signInDialog.getButton(DialogInterface.BUTTON_POSITIVE).performClick(); - TestUtils.awaitAsyncOperations(testExecutor); + TestUtils.awaitTaskFailure( + updateTask, AUTHENTICATION_CANCELED, ErrorMessages.AUTHENTICATION_CANCELED); verify(mockTesterSignInManager, times(1)).signInTester(); verify(mockNewReleaseFetcher, never()).checkForNewRelease(); - assertTaskFailure(updateTask, AUTHENTICATION_CANCELED, ErrorMessages.AUTHENTICATION_CANCELED); } @Test - public void updateIfNewReleaseAvailable_whenSignInFailed_checkForUpdateNotCalled() - throws InterruptedException { + public void updateIfNewReleaseAvailable_whenSignInFailed_checkForUpdateNotCalled() { when(mockSignInStorage.getSignInStatus()).thenReturn(false); when(mockTesterSignInManager.signInTester()) .thenReturn( @@ -366,51 +358,49 @@ public void updateIfNewReleaseAvailable_whenSignInFailed_checkForUpdateNotCalled AlertDialog signInDialog = assertAlertDialogShown(); signInDialog.getButton(DialogInterface.BUTTON_POSITIVE).performClick(); - TestUtils.awaitAsyncOperations(testExecutor); - assertTaskFailure(updateTask, AUTHENTICATION_FAILURE, AUTHENTICATION_ERROR); + TestUtils.awaitTaskFailure(updateTask, AUTHENTICATION_FAILURE, AUTHENTICATION_ERROR); } @Test public void updateIfNewReleaseAvailable_whenDialogDismissed_taskFails() throws InterruptedException { when(mockNewReleaseFetcher.checkForNewRelease()) - .thenReturn(Tasks.forResult(TEST_RELEASE_NEWER_AAB_INTERNAL.build())); + .thenReturn(Tasks.forResult(TEST_RELEASE_NEWER_AAB_INTERNAL)); UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable(); TestUtils.awaitAsyncOperations(testExecutor); AlertDialog updateDialog = assertAlertDialogShown(); updateDialog.getButton(AlertDialog.BUTTON_NEGATIVE).performClick(); // dismiss dialog - TestUtils.awaitAsyncOperations(testExecutor); + TestUtils.awaitTaskFailure(updateTask, INSTALLATION_CANCELED, UPDATE_CANCELED); assertFalse(updateDialog.isShowing()); - assertFalse(updateTask.isSuccessful()); - assertTaskFailure(updateTask, INSTALLATION_CANCELED, UPDATE_CANCELED); } @Test public void updateIfNewReleaseAvailable_whenDialogCanceled_taskFails() throws InterruptedException { when(mockNewReleaseFetcher.checkForNewRelease()) - .thenReturn(Tasks.forResult(TEST_RELEASE_NEWER_AAB_INTERNAL.build())); + .thenReturn(Tasks.forResult(TEST_RELEASE_NEWER_AAB_INTERNAL)); UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable(); - TestUtils.awaitAsyncOperations(testExecutor); + + // Task callbacks happen on the executor + TestUtils.awaitTermination(testExecutor); + + // Show update confirmation dialog happens on the UI thread + shadowOf(getMainLooper()).idle(); AlertDialog updateDialog = assertAlertDialogShown(); updateDialog.onBackPressed(); // cancels the dialog - TestUtils.awaitAsyncOperations(testExecutor); - TestUtils.awaitAsyncOperations(testExecutor); + TestUtils.awaitTaskFailure(updateTask, INSTALLATION_CANCELED, UPDATE_CANCELED); assertFalse(updateDialog.isShowing()); - - assertTaskFailure(updateTask, INSTALLATION_CANCELED, UPDATE_CANCELED); } @Test - public void updateIfNewReleaseAvailable_whenCheckForUpdateFails_updateAppNotCalled() - throws InterruptedException { + public void updateIfNewReleaseAvailable_whenCheckForUpdateFails_updateAppNotCalled() { when(mockNewReleaseFetcher.checkForNewRelease()) .thenReturn( Tasks.forException( @@ -420,60 +410,48 @@ public void updateIfNewReleaseAvailable_whenCheckForUpdateFails_updateAppNotCall UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable(); List progressEvents = new ArrayList<>(); - updateTask.addOnProgressListener(progressEvents::add); + updateTask.addOnProgressListener(FirebaseExecutors.directExecutor(), progressEvents::add); - TestUtils.awaitAsyncOperations(testExecutor); + TestUtils.awaitTaskFailure(updateTask, NETWORK_FAILURE, NETWORK_ERROR); assertEquals(1, progressEvents.size()); assertEquals(UpdateStatus.NEW_RELEASE_CHECK_FAILED, progressEvents.get(0).getUpdateStatus()); - verify(firebaseAppDistribution, never()).updateApp(); - assertTaskFailure(updateTask, NETWORK_FAILURE, NETWORK_ERROR); } @Test public void updateIfNewReleaseAvailable_whenTesterIsSignedIn_doesNotOpenDialog() - throws InterruptedException { + throws InterruptedException, FirebaseAppDistributionException, ExecutionException { + when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(null)); when(mockSignInStorage.getSignInStatus()).thenReturn(true); - firebaseAppDistribution.updateIfNewReleaseAvailable(); - TestUtils.awaitAsyncOperations(testExecutor); + UpdateTask task = firebaseAppDistribution.updateIfNewReleaseAvailable(); + TestUtils.awaitTask(task); assertNull(ShadowAlertDialog.getLatestAlertDialog()); } @Test - public void signInTester_whenDialogDismissed_taskFails() throws InterruptedException { + public void signInTester_whenDialogDismissed_taskFails() { when(mockSignInStorage.getSignInStatus()).thenReturn(false); Task updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable(); AlertDialog dialog = assertAlertDialogShown(); dialog.getButton(AlertDialog.BUTTON_NEGATIVE).performClick(); // dismiss dialog - TestUtils.awaitAsyncOperations(testExecutor); - assertFalse(updateTask.isSuccessful()); - Exception e = updateTask.getException(); - assertTrue(e instanceof FirebaseAppDistributionException); - assertEquals(AUTHENTICATION_CANCELED, ((FirebaseAppDistributionException) e).getErrorCode()); - assertEquals(ErrorMessages.AUTHENTICATION_CANCELED, e.getMessage()); + TestUtils.awaitTaskFailure( + updateTask, AUTHENTICATION_CANCELED, ErrorMessages.AUTHENTICATION_CANCELED); } @Test - public void updateIfNewReleaseAvailable_whenSignInDialogCanceled_taskFails() - throws InterruptedException { + public void updateIfNewReleaseAvailable_whenSignInDialogCanceled_taskFails() { when(mockSignInStorage.getSignInStatus()).thenReturn(false); Task signInTask = firebaseAppDistribution.updateIfNewReleaseAvailable(); AlertDialog dialog = assertAlertDialogShown(); dialog.onBackPressed(); // cancel dialog - TestUtils.awaitAsyncOperations(testExecutor); - TestUtils.awaitAsyncOperations(testExecutor); - assertTrue(signInTask.isComplete()); - assertFalse(signInTask.isSuccessful()); - Exception e = signInTask.getException(); - assertTrue(e instanceof FirebaseAppDistributionException); - assertEquals(AUTHENTICATION_CANCELED, ((FirebaseAppDistributionException) e).getErrorCode()); - assertEquals(ErrorMessages.AUTHENTICATION_CANCELED, e.getMessage()); + TestUtils.awaitTaskFailure( + signInTask, AUTHENTICATION_CANCELED, ErrorMessages.AUTHENTICATION_CANCELED); } private AlertDialog assertAlertDialogShown() { @@ -493,7 +471,7 @@ public void signOutTester_setsSignInStatusFalse() { @Test public void updateIfNewReleaseAvailable_receiveProgressUpdateFromUpdateApp() throws InterruptedException { - AppDistributionReleaseInternal newRelease = TEST_RELEASE_NEWER_AAB_INTERNAL.build(); + AppDistributionReleaseInternal newRelease = TEST_RELEASE_NEWER_AAB_INTERNAL; when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(newRelease)); UpdateTaskImpl updateTaskToReturn = new UpdateTaskImpl(); when(mockAabUpdater.updateAab(newRelease)).thenReturn(updateTaskToReturn); @@ -507,7 +485,7 @@ public void updateIfNewReleaseAvailable_receiveProgressUpdateFromUpdateApp() UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable(); List progressEvents = new ArrayList<>(); - updateTask.addOnProgressListener(progressEvents::add); + updateTask.addOnProgressListener(FirebaseExecutors.directExecutor(), progressEvents::add); TestUtils.awaitAsyncOperations(testExecutor); // Clicking the update button. @@ -535,13 +513,12 @@ public void updateIfNewReleaseAvailable_fromABackgroundThread_showsSignInDialog( } @Test - public void updateIfNewReleaseAvailable_whenScreenRotates_signInConfirmationDialogReappears() - throws InterruptedException { + public void updateIfNewReleaseAvailable_whenScreenRotates_signInConfirmationDialogReappears() { when(mockSignInStorage.getSignInStatus()).thenReturn(false); when(activity.isChangingConfigurations()).thenReturn(true); UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable(); - TestUtils.awaitAsyncOperations(testExecutor); + shadowOf(getMainLooper()).idle(); // Mimic activity recreation due to a configuration change firebaseAppDistribution.onActivityDestroyed(activity); @@ -554,7 +531,7 @@ public void updateIfNewReleaseAvailable_whenScreenRotates_signInConfirmationDial @Test public void updateIfNewReleaseAvailable_whenScreenRotates_updateDialogReappears() throws InterruptedException { - AppDistributionReleaseInternal newRelease = TEST_RELEASE_NEWER_AAB_INTERNAL.build(); + AppDistributionReleaseInternal newRelease = TEST_RELEASE_NEWER_AAB_INTERNAL; when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(newRelease)); when(activity.isChangingConfigurations()).thenReturn(true); @@ -571,8 +548,7 @@ public void updateIfNewReleaseAvailable_whenScreenRotates_updateDialogReappears( @Test public void - updateIfNewReleaseAvailable_whenSignInDialogShowingAndNewActivityStarts_signInTaskCancelled() - throws InterruptedException { + updateIfNewReleaseAvailable_whenSignInDialogShowingAndNewActivityStarts_signInTaskCancelled() { TestActivity testActivity2 = new TestActivity(); when(mockSignInStorage.getSignInStatus()).thenReturn(false); @@ -582,8 +558,7 @@ public void updateIfNewReleaseAvailable_whenScreenRotates_updateDialogReappears( firebaseAppDistribution.onActivityPaused(activity); firebaseAppDistribution.onActivityResumed(testActivity2); - TestUtils.awaitAsyncOperations(testExecutor); - assertTaskFailure( + TestUtils.awaitTaskFailure( updateTask, HOST_ACTIVITY_INTERRUPTED, ErrorMessages.HOST_ACTIVITY_INTERRUPTED); } @@ -592,7 +567,7 @@ public void updateIfNewReleaseAvailable_whenScreenRotates_updateDialogReappears( updateIfNewReleaseAvailable_whenUpdateDialogShowingAndNewActivityStarts_updateTaskCancelled() throws InterruptedException { TestActivity testActivity2 = new TestActivity(); - AppDistributionReleaseInternal newRelease = TEST_RELEASE_NEWER_AAB_INTERNAL.build(); + AppDistributionReleaseInternal newRelease = TEST_RELEASE_NEWER_AAB_INTERNAL; when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(newRelease)); UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable(); @@ -601,26 +576,25 @@ public void updateIfNewReleaseAvailable_whenScreenRotates_updateDialogReappears( // Mimic different activity getting resumed firebaseAppDistribution.onActivityPaused(activity); firebaseAppDistribution.onActivityResumed(testActivity2); - TestUtils.awaitAsyncOperations(testExecutor); - assertTaskFailure( + TestUtils.awaitTaskFailure( updateTask, HOST_ACTIVITY_INTERRUPTED, ErrorMessages.HOST_ACTIVITY_INTERRUPTED); } @Test - public void updateAppTask_whenNoReleaseAvailable_throwsError() throws InterruptedException { + public void updateAppTask_whenNoReleaseAvailable_throwsError() { firebaseAppDistribution.getCachedNewRelease().set(null); when(mockSignInStorage.getSignInStatus()).thenReturn(true); UpdateTask updateTask = firebaseAppDistribution.updateApp(); - TestUtils.awaitAsyncOperations(testExecutor); - assertTaskFailure(updateTask, UPDATE_NOT_AVAILABLE, RELEASE_NOT_FOUND_ERROR); + TestUtils.awaitTaskFailure(updateTask, UPDATE_NOT_AVAILABLE, RELEASE_NOT_FOUND_ERROR); } @Test - public void updateApp_withAabReleaseAvailable_returnsSameAabTask() throws InterruptedException { - AppDistributionReleaseInternal release = TEST_RELEASE_NEWER_AAB_INTERNAL.build(); + public void updateApp_withAabReleaseAvailable_returnsSameAabTask() + throws InterruptedException, FirebaseAppDistributionException, ExecutionException { + AppDistributionReleaseInternal release = TEST_RELEASE_NEWER_AAB_INTERNAL; firebaseAppDistribution.getCachedNewRelease().set(release); UpdateTaskImpl updateTaskToReturn = new UpdateTaskImpl(); doReturn(updateTaskToReturn).when(mockAabUpdater).updateAab(release); @@ -631,16 +605,17 @@ public void updateApp_withAabReleaseAvailable_returnsSameAabTask() throws Interr // Complete original task updateTaskToReturn.setResult(); - TestUtils.awaitAsyncOperations(testExecutor); + TestUtils.awaitTask(updateTask); // Returned task is complete assertTrue(updateTask.isSuccessful()); } @Test - public void updateApp_withApkReleaseAvailable_returnsSameApkTask() throws InterruptedException { + public void updateApp_withApkReleaseAvailable_returnsSameApkTask() + throws InterruptedException, FirebaseAppDistributionException, ExecutionException { when(mockSignInStorage.getSignInStatus()).thenReturn(true); - AppDistributionReleaseInternal release = TEST_RELEASE_NEWER_APK_INTERNAL.build(); + AppDistributionReleaseInternal release = TEST_RELEASE_NEWER_APK_INTERNAL; firebaseAppDistribution.getCachedNewRelease().set(release); UpdateTaskImpl updateTaskToReturn = new UpdateTaskImpl(); doReturn(updateTaskToReturn).when(mockApkUpdater).updateApk(release, false); @@ -650,7 +625,7 @@ public void updateApp_withApkReleaseAvailable_returnsSameApkTask() throws Interr // Complete original task updateTaskToReturn.setResult(); - TestUtils.awaitAsyncOperations(testExecutor); + TestUtils.awaitTask(updateTask); // Returned task is complete assertTrue(updateTask.isSuccessful()); diff --git a/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/TestUtils.java b/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/TestUtils.java index 15241d6fb1e..a697d600248 100644 --- a/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/TestUtils.java +++ b/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/TestUtils.java @@ -82,9 +82,13 @@ static T awaitTask(Task task) return onCompleteListener.await(); } + static void awaitTermination(ExecutorService executorService) throws InterruptedException { + executorService.awaitTermination(100, TimeUnit.MILLISECONDS); + } + static void awaitAsyncOperations(ExecutorService executorService) throws InterruptedException { // Await anything enqueued to the executor - executorService.awaitTermination(100, TimeUnit.MILLISECONDS); + awaitTermination(executorService); // Idle the main looper, which is also running these tests, so any Task or lifecycle callbacks // can be handled. See http://robolectric.org/blog/2019/06/04/paused-looper/ for more info. From 1472f3e4d52d64507e1d6a7e176b08cce62bd986 Mon Sep 17 00:00:00 2001 From: Lee Kellogg Date: Thu, 8 Dec 2022 19:04:49 -0500 Subject: [PATCH 09/12] Update registrar --- .../impl/FirebaseAppDistributionRegistrar.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionRegistrar.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionRegistrar.java index c11d2f98d28..0670ba7cb61 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionRegistrar.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionRegistrar.java @@ -57,10 +57,7 @@ public class FirebaseAppDistributionRegistrar implements ComponentRegistrar { .add(Dependency.requiredProvider(FirebaseInstallationsApi.class)) .add(Dependency.required(blockingExecutor)) .add(Dependency.required(lightweightExecutor)) - .factory( - c -> - buildFirebaseAppDistribution( - c, c.get(blockingExecutor), c.get(lightweightExecutor))) + .factory(c -> buildFirebaseAppDistribution(c, blockingExecutor, lightweightExecutor)) // construct FirebaseAppDistribution instance on startup so we can register for // activity lifecycle callbacks before the API is called .alwaysEager() @@ -69,8 +66,12 @@ public class FirebaseAppDistributionRegistrar implements ComponentRegistrar { } private FirebaseAppDistribution buildFirebaseAppDistribution( - ComponentContainer container, Executor blockingExecutor, Executor lightweightExecutor) { + ComponentContainer container, + Qualified blockingExecutorType, + Qualified lightweightExecutorType) { FirebaseApp firebaseApp = container.get(FirebaseApp.class); + Executor blockingExecutor = container.get(blockingExecutorType); + Executor lightweightExecutor = container.get(lightweightExecutorType); Context context = firebaseApp.getApplicationContext(); Provider firebaseInstallationsApiProvider = container.getProvider(FirebaseInstallationsApi.class); From 0b6a9774fbfc43e9d8cbf385f7f65e7f4a637581 Mon Sep 17 00:00:00 2001 From: Lee Kellogg Date: Fri, 9 Dec 2022 08:15:23 -0500 Subject: [PATCH 10/12] Fix test and add static imports from TestUtils --- ...irebaseAppDistributionServiceImplTest.java | 75 ++++++++++--------- 1 file changed, 39 insertions(+), 36 deletions(-) diff --git a/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java b/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java index 904b17c4ce7..2da6685c75c 100644 --- a/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java +++ b/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java @@ -26,6 +26,11 @@ import static com.google.firebase.appdistribution.impl.ErrorMessages.NETWORK_ERROR; import static com.google.firebase.appdistribution.impl.ErrorMessages.RELEASE_NOT_FOUND_ERROR; import static com.google.firebase.appdistribution.impl.ErrorMessages.UPDATE_CANCELED; +import static com.google.firebase.appdistribution.impl.TestUtils.awaitAsyncOperations; +import static com.google.firebase.appdistribution.impl.TestUtils.awaitTask; +import static com.google.firebase.appdistribution.impl.TestUtils.awaitTaskFailure; +import static com.google.firebase.appdistribution.impl.TestUtils.awaitTermination; +import static com.google.firebase.appdistribution.impl.TestUtils.mockForegroundActivity; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; @@ -181,7 +186,7 @@ public void setup() throws FirebaseAppDistributionException { shadowPackageManager.installPackage(packageInfo); activity = spy(Robolectric.buildActivity(TestActivity.class).create().get()); - TestUtils.mockForegroundActivity(mockLifecycleNotifier, activity); + mockForegroundActivity(mockLifecycleNotifier, activity); } @Test @@ -194,7 +199,7 @@ public void checkForNewRelease_whenCheckForNewReleaseFails_throwsError() { Task task = firebaseAppDistribution.checkForNewRelease(); - TestUtils.awaitTaskFailure(task, NETWORK_FAILURE, JSON_PARSING_ERROR); + awaitTaskFailure(task, NETWORK_FAILURE, JSON_PARSING_ERROR); } @Test @@ -203,7 +208,7 @@ public void checkForNewRelease_testerIsNotSignedIn_taskFails() { Task task = firebaseAppDistribution.checkForNewRelease(); - TestUtils.awaitTaskFailure(task, AUTHENTICATION_FAILURE, "Tester is not signed in"); + awaitTaskFailure(task, AUTHENTICATION_FAILURE, "Tester is not signed in"); } @Test @@ -213,7 +218,7 @@ public void checkForNewRelease_whenCheckForNewReleaseSucceeds_returnsRelease() when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(internalRelease)); Task task = firebaseAppDistribution.checkForNewRelease(); - TestUtils.awaitTask(task); + awaitTask(task); assertEquals(TEST_RELEASE_NEWER_AAB, task.getResult()); assertEquals(internalRelease, firebaseAppDistribution.getCachedNewRelease().getSnapshot()); @@ -228,8 +233,8 @@ public void checkForNewRelease_authenticationFailure_signOutTester() throws Inte Task task = firebaseAppDistribution.checkForNewRelease(); - TestUtils.awaitTaskFailure(task, AUTHENTICATION_FAILURE, "Test"); - TestUtils.awaitTermination(testExecutor); + awaitTaskFailure(task, AUTHENTICATION_FAILURE, "Test"); + awaitTermination(testExecutor); verify(mockSignInStorage, times(1)).setSignInStatus(false); } @@ -239,7 +244,7 @@ public void updateApp_whenNotSignedIn_throwsError() { UpdateTask updateTask = firebaseAppDistribution.updateApp(); - TestUtils.awaitTaskFailure(updateTask, AUTHENTICATION_FAILURE, "Tester is not signed in"); + awaitTaskFailure(updateTask, AUTHENTICATION_FAILURE, "Tester is not signed in"); } @Test @@ -249,7 +254,7 @@ public void updateIfNewReleaseAvailable_whenNewAabReleaseAvailable_showsUpdateDi .thenReturn(Tasks.forResult((TEST_RELEASE_NEWER_AAB_INTERNAL))); firebaseAppDistribution.updateIfNewReleaseAvailable(); - TestUtils.awaitAsyncOperations(testExecutor); + awaitAsyncOperations(testExecutor); AlertDialog dialog = assertAlertDialogShown(); assertEquals( @@ -269,7 +274,7 @@ public void updateIfNewReleaseAvailable_fromABackgroundThread_showsUpdateDialog( ExecutorService executorService = Executors.newSingleThreadExecutor(); executorService.submit(() -> firebaseAppDistribution.updateIfNewReleaseAvailable()); - TestUtils.awaitAsyncOperations(executorService); + awaitAsyncOperations(executorService); assertAlertDialogShown(); } @@ -283,7 +288,7 @@ public void updateIfNewReleaseAvailable_whenReleaseNotesEmpty_doesNotShowRelease (TEST_RELEASE_NEWER_AAB_INTERNAL.toBuilder().setReleaseNotes("").build()))); firebaseAppDistribution.updateIfNewReleaseAvailable(); - TestUtils.awaitAsyncOperations(testExecutor); + awaitAsyncOperations(testExecutor); AlertDialog dialog = assertAlertDialogShown(); assertEquals( @@ -302,7 +307,7 @@ public void updateIfNewReleaseAvailable_whenNoReleaseAvailable_updateDialogNotSh List progressEvents = new ArrayList<>(); task.addOnProgressListener(FirebaseExecutors.directExecutor(), progressEvents::add); - TestUtils.awaitTermination(testExecutor); + awaitTermination(testExecutor); assertEquals(1, progressEvents.size()); assertEquals(UpdateStatus.NEW_RELEASE_NOT_AVAILABLE, progressEvents.get(0).getUpdateStatus()); @@ -318,7 +323,7 @@ public void updateIfNewReleaseAvailable_whenActivityBackgrounded_updateDialogNot List progressEvents = new ArrayList<>(); task.addOnProgressListener(FirebaseExecutors.directExecutor(), progressEvents::add); - TestUtils.awaitTermination(testExecutor); + awaitTermination(testExecutor); assertEquals(1, progressEvents.size()); assertEquals(UpdateStatus.NEW_RELEASE_NOT_AVAILABLE, progressEvents.get(0).getUpdateStatus()); @@ -339,8 +344,7 @@ public void updateIfNewReleaseAvailable_whenSignInCancelled_checkForUpdateNotCal AlertDialog signInDialog = assertAlertDialogShown(); signInDialog.getButton(DialogInterface.BUTTON_POSITIVE).performClick(); - TestUtils.awaitTaskFailure( - updateTask, AUTHENTICATION_CANCELED, ErrorMessages.AUTHENTICATION_CANCELED); + awaitTaskFailure(updateTask, AUTHENTICATION_CANCELED, ErrorMessages.AUTHENTICATION_CANCELED); verify(mockTesterSignInManager, times(1)).signInTester(); verify(mockNewReleaseFetcher, never()).checkForNewRelease(); } @@ -359,7 +363,7 @@ public void updateIfNewReleaseAvailable_whenSignInFailed_checkForUpdateNotCalled AlertDialog signInDialog = assertAlertDialogShown(); signInDialog.getButton(DialogInterface.BUTTON_POSITIVE).performClick(); - TestUtils.awaitTaskFailure(updateTask, AUTHENTICATION_FAILURE, AUTHENTICATION_ERROR); + awaitTaskFailure(updateTask, AUTHENTICATION_FAILURE, AUTHENTICATION_ERROR); } @Test @@ -369,12 +373,12 @@ public void updateIfNewReleaseAvailable_whenDialogDismissed_taskFails() .thenReturn(Tasks.forResult(TEST_RELEASE_NEWER_AAB_INTERNAL)); UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable(); - TestUtils.awaitAsyncOperations(testExecutor); + awaitAsyncOperations(testExecutor); AlertDialog updateDialog = assertAlertDialogShown(); updateDialog.getButton(AlertDialog.BUTTON_NEGATIVE).performClick(); // dismiss dialog - TestUtils.awaitTaskFailure(updateTask, INSTALLATION_CANCELED, UPDATE_CANCELED); + awaitTaskFailure(updateTask, INSTALLATION_CANCELED, UPDATE_CANCELED); assertFalse(updateDialog.isShowing()); } @@ -387,7 +391,7 @@ public void updateIfNewReleaseAvailable_whenDialogCanceled_taskFails() UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable(); // Task callbacks happen on the executor - TestUtils.awaitTermination(testExecutor); + awaitTermination(testExecutor); // Show update confirmation dialog happens on the UI thread shadowOf(getMainLooper()).idle(); @@ -395,7 +399,7 @@ public void updateIfNewReleaseAvailable_whenDialogCanceled_taskFails() AlertDialog updateDialog = assertAlertDialogShown(); updateDialog.onBackPressed(); // cancels the dialog - TestUtils.awaitTaskFailure(updateTask, INSTALLATION_CANCELED, UPDATE_CANCELED); + awaitTaskFailure(updateTask, INSTALLATION_CANCELED, UPDATE_CANCELED); assertFalse(updateDialog.isShowing()); } @@ -412,7 +416,7 @@ public void updateIfNewReleaseAvailable_whenCheckForUpdateFails_updateAppNotCall List progressEvents = new ArrayList<>(); updateTask.addOnProgressListener(FirebaseExecutors.directExecutor(), progressEvents::add); - TestUtils.awaitTaskFailure(updateTask, NETWORK_FAILURE, NETWORK_ERROR); + awaitTaskFailure(updateTask, NETWORK_FAILURE, NETWORK_ERROR); assertEquals(1, progressEvents.size()); assertEquals(UpdateStatus.NEW_RELEASE_CHECK_FAILED, progressEvents.get(0).getUpdateStatus()); verify(firebaseAppDistribution, never()).updateApp(); @@ -425,7 +429,7 @@ public void updateIfNewReleaseAvailable_whenTesterIsSignedIn_doesNotOpenDialog() when(mockSignInStorage.getSignInStatus()).thenReturn(true); UpdateTask task = firebaseAppDistribution.updateIfNewReleaseAvailable(); - TestUtils.awaitTask(task); + awaitTask(task); assertNull(ShadowAlertDialog.getLatestAlertDialog()); } @@ -438,8 +442,7 @@ public void signInTester_whenDialogDismissed_taskFails() { AlertDialog dialog = assertAlertDialogShown(); dialog.getButton(AlertDialog.BUTTON_NEGATIVE).performClick(); // dismiss dialog - TestUtils.awaitTaskFailure( - updateTask, AUTHENTICATION_CANCELED, ErrorMessages.AUTHENTICATION_CANCELED); + awaitTaskFailure(updateTask, AUTHENTICATION_CANCELED, ErrorMessages.AUTHENTICATION_CANCELED); } @Test @@ -450,8 +453,7 @@ public void updateIfNewReleaseAvailable_whenSignInDialogCanceled_taskFails() { AlertDialog dialog = assertAlertDialogShown(); dialog.onBackPressed(); // cancel dialog - TestUtils.awaitTaskFailure( - signInTask, AUTHENTICATION_CANCELED, ErrorMessages.AUTHENTICATION_CANCELED); + awaitTaskFailure(signInTask, AUTHENTICATION_CANCELED, ErrorMessages.AUTHENTICATION_CANCELED); } private AlertDialog assertAlertDialogShown() { @@ -463,8 +465,9 @@ private AlertDialog assertAlertDialogShown() { } @Test - public void signOutTester_setsSignInStatusFalse() { + public void signOutTester_setsSignInStatusFalse() throws InterruptedException { firebaseAppDistribution.signOutTester(); + awaitTermination(testExecutor); verify(mockSignInStorage).setSignInStatus(false); } @@ -486,12 +489,12 @@ public void updateIfNewReleaseAvailable_receiveProgressUpdateFromUpdateApp() List progressEvents = new ArrayList<>(); updateTask.addOnProgressListener(FirebaseExecutors.directExecutor(), progressEvents::add); - TestUtils.awaitAsyncOperations(testExecutor); + awaitAsyncOperations(testExecutor); // Clicking the update button. AlertDialog updateDialog = (AlertDialog) ShadowAlertDialog.getLatestDialog(); updateDialog.getButton(Dialog.BUTTON_POSITIVE).performClick(); - TestUtils.awaitAsyncOperations(testExecutor); + awaitAsyncOperations(testExecutor); // Update flow assertEquals(1, progressEvents.size()); @@ -506,7 +509,7 @@ public void updateIfNewReleaseAvailable_fromABackgroundThread_showsSignInDialog( ExecutorService executorService = Executors.newSingleThreadExecutor(); Future future = executorService.submit(() -> firebaseAppDistribution.updateIfNewReleaseAvailable()); - TestUtils.awaitAsyncOperations(executorService); + awaitAsyncOperations(executorService); assertAlertDialogShown(); assertFalse(((UpdateTask) future.get()).isComplete()); @@ -536,7 +539,7 @@ public void updateIfNewReleaseAvailable_whenScreenRotates_updateDialogReappears( when(activity.isChangingConfigurations()).thenReturn(true); UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable(); - TestUtils.awaitAsyncOperations(testExecutor); + awaitAsyncOperations(testExecutor); // Mimic activity recreation due to a configuration change firebaseAppDistribution.onActivityDestroyed(activity); @@ -558,7 +561,7 @@ public void updateIfNewReleaseAvailable_whenScreenRotates_updateDialogReappears( firebaseAppDistribution.onActivityPaused(activity); firebaseAppDistribution.onActivityResumed(testActivity2); - TestUtils.awaitTaskFailure( + awaitTaskFailure( updateTask, HOST_ACTIVITY_INTERRUPTED, ErrorMessages.HOST_ACTIVITY_INTERRUPTED); } @@ -571,13 +574,13 @@ public void updateIfNewReleaseAvailable_whenScreenRotates_updateDialogReappears( when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(newRelease)); UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable(); - TestUtils.awaitAsyncOperations(testExecutor); + awaitAsyncOperations(testExecutor); // Mimic different activity getting resumed firebaseAppDistribution.onActivityPaused(activity); firebaseAppDistribution.onActivityResumed(testActivity2); - TestUtils.awaitTaskFailure( + awaitTaskFailure( updateTask, HOST_ACTIVITY_INTERRUPTED, ErrorMessages.HOST_ACTIVITY_INTERRUPTED); } @@ -588,7 +591,7 @@ public void updateAppTask_whenNoReleaseAvailable_throwsError() { UpdateTask updateTask = firebaseAppDistribution.updateApp(); - TestUtils.awaitTaskFailure(updateTask, UPDATE_NOT_AVAILABLE, RELEASE_NOT_FOUND_ERROR); + awaitTaskFailure(updateTask, UPDATE_NOT_AVAILABLE, RELEASE_NOT_FOUND_ERROR); } @Test @@ -605,7 +608,7 @@ public void updateApp_withAabReleaseAvailable_returnsSameAabTask() // Complete original task updateTaskToReturn.setResult(); - TestUtils.awaitTask(updateTask); + awaitTask(updateTask); // Returned task is complete assertTrue(updateTask.isSuccessful()); @@ -625,7 +628,7 @@ public void updateApp_withApkReleaseAvailable_returnsSameApkTask() // Complete original task updateTaskToReturn.setResult(); - TestUtils.awaitTask(updateTask); + awaitTask(updateTask); // Returned task is complete assertTrue(updateTask.isSuccessful()); From e1bd32972c8fdab6f0742f6f6b5628eb477ce1a3 Mon Sep 17 00:00:00 2001 From: Lee Kellogg Date: Fri, 9 Dec 2022 12:40:31 -0500 Subject: [PATCH 11/12] Incorporate some feedback --- .../impl/FirebaseAppDistributionImpl.java | 13 ++++----- .../impl/SequentialReference.java | 29 ++++--------------- ...irebaseAppDistributionServiceImplTest.java | 4 ++- 3 files changed, 15 insertions(+), 31 deletions(-) diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java index 2654b55c244..a37f12c26f3 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java @@ -32,6 +32,7 @@ import com.google.android.gms.tasks.TaskCompletionSource; import com.google.android.gms.tasks.Tasks; import com.google.firebase.FirebaseApp; +import com.google.firebase.annotations.concurrent.Lightweight; import com.google.firebase.appdistribution.AppDistributionRelease; import com.google.firebase.appdistribution.BinaryType; import com.google.firebase.appdistribution.FirebaseAppDistribution; @@ -40,7 +41,6 @@ import com.google.firebase.appdistribution.UpdateProgress; import com.google.firebase.appdistribution.UpdateStatus; import com.google.firebase.appdistribution.UpdateTask; -import com.google.firebase.concurrent.FirebaseExecutors; import java.util.concurrent.Executor; /** @@ -61,7 +61,7 @@ class FirebaseAppDistributionImpl implements FirebaseAppDistribution { private final SequentialReference cachedNewRelease; private TaskCache updateIfNewReleaseAvailableTaskCache = new TaskCache<>(); private TaskCache> checkForNewReleaseTaskCache = new TaskCache<>(); - private Executor lightweightExecutor; + @Lightweight private Executor lightweightExecutor; private AlertDialog updateConfirmationDialog; private AlertDialog signInConfirmationDialog; @Nullable private Activity dialogHostActivity = null; @@ -81,7 +81,7 @@ class FirebaseAppDistributionImpl implements FirebaseAppDistribution { @NonNull AabUpdater aabUpdater, @NonNull SignInStorage signInStorage, @NonNull FirebaseAppDistributionLifecycleNotifier lifecycleNotifier, - @NonNull Executor lightweightExecutor) { + @NonNull @Lightweight Executor lightweightExecutor) { this.firebaseApp = firebaseApp; this.testerSignInManager = testerSignInManager; this.newReleaseFetcher = newReleaseFetcher; @@ -89,7 +89,7 @@ class FirebaseAppDistributionImpl implements FirebaseAppDistribution { this.aabUpdater = aabUpdater; this.signInStorage = signInStorage; this.lifecycleNotifier = lifecycleNotifier; - this.cachedNewRelease = SequentialReference.withBaseExecutor(lightweightExecutor); + this.cachedNewRelease = new SequentialReference<>(lightweightExecutor); this.lightweightExecutor = lightweightExecutor; lifecycleNotifier.addOnActivityDestroyedListener(this::onActivityDestroyed); lifecycleNotifier.addOnActivityPausedListener(this::onActivityPaused); @@ -220,8 +220,7 @@ public Task signInTester() { public void signOutTester() { cachedNewRelease .set(null) - .addOnSuccessListener( - FirebaseExecutors.directExecutor(), unused -> signInStorage.setSignInStatus(false)); + .addOnSuccessListener(lightweightExecutor, unused -> signInStorage.setSignInStatus(false)); } @Override @@ -240,7 +239,7 @@ public synchronized Task checkForNewRelease() { .checkForNewRelease() .onSuccessTask(lightweightExecutor, release -> cachedNewRelease.set(release)) .onSuccessTask( - FirebaseExecutors.directExecutor(), + lightweightExecutor, release -> Tasks.forResult(ReleaseUtils.convertToAppDistributionRelease(release))) .addOnFailureListener( diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java index c6ebf93e2f8..9d200777a0f 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/SequentialReference.java @@ -14,8 +14,6 @@ package com.google.firebase.appdistribution.impl; -import androidx.annotation.VisibleForTesting; -import com.google.android.gms.tasks.OnSuccessListener; import com.google.android.gms.tasks.Task; import com.google.android.gms.tasks.TaskCompletionSource; import com.google.firebase.concurrent.FirebaseExecutors; @@ -30,21 +28,12 @@ class SequentialReference { private final Executor sequentialExecutor; private T value; - /** Get a {@link SequentialReference} that controls access uses the given sequential executor. */ - static SequentialReference withSequentialExecutor(Executor sequentialExecutor) { - return new SequentialReference(sequentialExecutor); - } - /** - * Get a {@link SequentialReference} that controls access using its own sequential executor backed - * by the given base executor. + * Constructor for a {@link SequentialReference} that controls access using its own sequential + * executor backed by the given base executor. */ - static SequentialReference withBaseExecutor(Executor baseExecutor) { - return new SequentialReference(FirebaseExecutors.newSequentialExecutor(baseExecutor)); - } - - private SequentialReference(Executor sequentialExecutor) { - this.sequentialExecutor = sequentialExecutor; + SequentialReference(Executor baseExecutor) { + this.sequentialExecutor = FirebaseExecutors.newSequentialExecutor(baseExecutor); } /** @@ -65,18 +54,12 @@ Task set(T newValue) { /** * Gets a {@link Task} that will complete with the value. * - *

Unless a direct executer is passed to {@link Task#addOnSuccessListener(Executor, - * OnSuccessListener)} or similar methods, be aware that the value may have changed by the time it - * is used. + *

In some cases the value may have changed by the time any callbacks attached to the returned + * {@link Task} are called. */ Task get() { TaskCompletionSource taskCompletionSource = new TaskCompletionSource<>(); sequentialExecutor.execute(() -> taskCompletionSource.setResult(value)); return taskCompletionSource.getTask(); } - - @VisibleForTesting - T getSnapshot() { - return value; - } } diff --git a/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java b/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java index 2da6685c75c..b700e5217ea 100644 --- a/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java +++ b/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java @@ -221,7 +221,9 @@ public void checkForNewRelease_whenCheckForNewReleaseSucceeds_returnsRelease() awaitTask(task); assertEquals(TEST_RELEASE_NEWER_AAB, task.getResult()); - assertEquals(internalRelease, firebaseAppDistribution.getCachedNewRelease().getSnapshot()); + AppDistributionReleaseInternal cachedNewRelease = + awaitTask(firebaseAppDistribution.getCachedNewRelease().get()); + assertEquals(internalRelease, cachedNewRelease); } @Test From 9ef0e50a71dae3f7bea8bcbc89bca7e94ec7f325 Mon Sep 17 00:00:00 2001 From: Lee Kellogg Date: Fri, 9 Dec 2022 13:56:28 -0500 Subject: [PATCH 12/12] Use actual lightweight executor in test --- .../firebase-appdistribution.gradle | 1 + .../impl/FirebaseAppDistributionImpl.java | 1 + ...irebaseAppDistributionServiceImplTest.java | 29 ++++++++++--------- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/firebase-appdistribution/firebase-appdistribution.gradle b/firebase-appdistribution/firebase-appdistribution.gradle index 47557c0dfb8..79e894ef741 100644 --- a/firebase-appdistribution/firebase-appdistribution.gradle +++ b/firebase-appdistribution/firebase-appdistribution.gradle @@ -49,6 +49,7 @@ dependencies { implementation project(':firebase-installations-interop') implementation project(':firebase-common') testImplementation project(path: ':firebase-appdistribution') + testImplementation project(':integ-testing') runtimeOnly project(':firebase-installations') testImplementation 'junit:junit:4.13.2' diff --git a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java index a37f12c26f3..1080d3408cd 100644 --- a/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java +++ b/firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java @@ -320,6 +320,7 @@ void onActivityResumed(Activity activity) { cachedNewRelease .get() .addOnSuccessListener( + lightweightExecutor, release -> showUpdateConfirmationDialog( activity, ReleaseUtils.convertToAppDistributionRelease(release))); diff --git a/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java b/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java index b700e5217ea..6101eb02225 100644 --- a/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java +++ b/firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionServiceImplTest.java @@ -65,6 +65,7 @@ import com.google.firebase.appdistribution.UpdateStatus; import com.google.firebase.appdistribution.UpdateTask; import com.google.firebase.concurrent.FirebaseExecutors; +import com.google.firebase.concurrent.TestOnlyExecutors; import com.google.firebase.installations.InstallationTokenResult; import java.util.ArrayList; import java.util.List; @@ -119,7 +120,7 @@ public class FirebaseAppDistributionServiceImplTest { .setDownloadUrl(TEST_URL) .build(); - private final ExecutorService testExecutor = Executors.newSingleThreadExecutor(); + private final ExecutorService lightweightExecutor = TestOnlyExecutors.lite(); private FirebaseAppDistributionImpl firebaseAppDistribution; private TestActivity activity; @@ -161,7 +162,7 @@ public void setup() throws FirebaseAppDistributionException { mockAabUpdater, mockSignInStorage, mockLifecycleNotifier, - testExecutor)); + lightweightExecutor)); when(mockTesterSignInManager.signInTester()).thenReturn(Tasks.forResult(null)); when(mockSignInStorage.getSignInStatus()).thenReturn(true); @@ -236,7 +237,7 @@ public void checkForNewRelease_authenticationFailure_signOutTester() throws Inte Task task = firebaseAppDistribution.checkForNewRelease(); awaitTaskFailure(task, AUTHENTICATION_FAILURE, "Test"); - awaitTermination(testExecutor); + awaitTermination(lightweightExecutor); verify(mockSignInStorage, times(1)).setSignInStatus(false); } @@ -256,7 +257,7 @@ public void updateIfNewReleaseAvailable_whenNewAabReleaseAvailable_showsUpdateDi .thenReturn(Tasks.forResult((TEST_RELEASE_NEWER_AAB_INTERNAL))); firebaseAppDistribution.updateIfNewReleaseAvailable(); - awaitAsyncOperations(testExecutor); + awaitAsyncOperations(lightweightExecutor); AlertDialog dialog = assertAlertDialogShown(); assertEquals( @@ -290,7 +291,7 @@ public void updateIfNewReleaseAvailable_whenReleaseNotesEmpty_doesNotShowRelease (TEST_RELEASE_NEWER_AAB_INTERNAL.toBuilder().setReleaseNotes("").build()))); firebaseAppDistribution.updateIfNewReleaseAvailable(); - awaitAsyncOperations(testExecutor); + awaitAsyncOperations(lightweightExecutor); AlertDialog dialog = assertAlertDialogShown(); assertEquals( @@ -309,7 +310,7 @@ public void updateIfNewReleaseAvailable_whenNoReleaseAvailable_updateDialogNotSh List progressEvents = new ArrayList<>(); task.addOnProgressListener(FirebaseExecutors.directExecutor(), progressEvents::add); - awaitTermination(testExecutor); + awaitTermination(lightweightExecutor); assertEquals(1, progressEvents.size()); assertEquals(UpdateStatus.NEW_RELEASE_NOT_AVAILABLE, progressEvents.get(0).getUpdateStatus()); @@ -325,7 +326,7 @@ public void updateIfNewReleaseAvailable_whenActivityBackgrounded_updateDialogNot List progressEvents = new ArrayList<>(); task.addOnProgressListener(FirebaseExecutors.directExecutor(), progressEvents::add); - awaitTermination(testExecutor); + awaitTermination(lightweightExecutor); assertEquals(1, progressEvents.size()); assertEquals(UpdateStatus.NEW_RELEASE_NOT_AVAILABLE, progressEvents.get(0).getUpdateStatus()); @@ -375,7 +376,7 @@ public void updateIfNewReleaseAvailable_whenDialogDismissed_taskFails() .thenReturn(Tasks.forResult(TEST_RELEASE_NEWER_AAB_INTERNAL)); UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable(); - awaitAsyncOperations(testExecutor); + awaitAsyncOperations(lightweightExecutor); AlertDialog updateDialog = assertAlertDialogShown(); updateDialog.getButton(AlertDialog.BUTTON_NEGATIVE).performClick(); // dismiss dialog @@ -393,7 +394,7 @@ public void updateIfNewReleaseAvailable_whenDialogCanceled_taskFails() UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable(); // Task callbacks happen on the executor - awaitTermination(testExecutor); + awaitTermination(lightweightExecutor); // Show update confirmation dialog happens on the UI thread shadowOf(getMainLooper()).idle(); @@ -469,7 +470,7 @@ private AlertDialog assertAlertDialogShown() { @Test public void signOutTester_setsSignInStatusFalse() throws InterruptedException { firebaseAppDistribution.signOutTester(); - awaitTermination(testExecutor); + awaitTermination(lightweightExecutor); verify(mockSignInStorage).setSignInStatus(false); } @@ -491,12 +492,12 @@ public void updateIfNewReleaseAvailable_receiveProgressUpdateFromUpdateApp() List progressEvents = new ArrayList<>(); updateTask.addOnProgressListener(FirebaseExecutors.directExecutor(), progressEvents::add); - awaitAsyncOperations(testExecutor); + awaitAsyncOperations(lightweightExecutor); // Clicking the update button. AlertDialog updateDialog = (AlertDialog) ShadowAlertDialog.getLatestDialog(); updateDialog.getButton(Dialog.BUTTON_POSITIVE).performClick(); - awaitAsyncOperations(testExecutor); + awaitAsyncOperations(lightweightExecutor); // Update flow assertEquals(1, progressEvents.size()); @@ -541,7 +542,7 @@ public void updateIfNewReleaseAvailable_whenScreenRotates_updateDialogReappears( when(activity.isChangingConfigurations()).thenReturn(true); UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable(); - awaitAsyncOperations(testExecutor); + awaitAsyncOperations(lightweightExecutor); // Mimic activity recreation due to a configuration change firebaseAppDistribution.onActivityDestroyed(activity); @@ -576,7 +577,7 @@ public void updateIfNewReleaseAvailable_whenScreenRotates_updateDialogReappears( when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(newRelease)); UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable(); - awaitAsyncOperations(testExecutor); + awaitAsyncOperations(lightweightExecutor); // Mimic different activity getting resumed firebaseAppDistribution.onActivityPaused(activity);